Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Resize and RelativeResize filters #37

Merged
merged 1 commit into from
Nov 16, 2011
Merged

Add Resize and RelativeResize filters #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions Imagine/Filter/Loader/RelativeResizeFilterLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

namespace Liip\ImagineBundle\Imagine\Filter\Loader;

use Imagine\Exception\InvalidArgumentException;
use Imagine\Image\ImageInterface;
use Liip\ImagineBundle\Imagine\Filter\RelativeResize;

/**
* Loader for this bundle's relative resize filter.
*
* @author Jeremy Mikola <jmikola@gmail.com>
*/
class RelativeResizeFilterLoader implements LoaderInterface
{
/**
* @see Liip\ImagineBundle\Imagine\Filter\Loader\LoaderInterface::load()
*/
public function load(ImageInterface $image, array $options = array())
{
foreach ($options as $method => $parameter) {
$filter = new RelativeResize($method, $parameter);

return $filter->apply($image);
}

throw new InvalidArgumentException('Expected method/parameter pair, none given');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like a really weird way to configure the filter. What if the user sets several entries in the option array ?

another issue is that it introduces a limitation in how it can be used from Twig: hash keys cannot be set dynamically with variables so it is impossible to set the method according to a parameter (which could be useful when making it configurable). Why not using array('method' => $method, 'parameter' => $parameter) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't consider how filter parameters are defined for LiipImaigneBundle. For the original bundle, I have the following in my yaml configuration:

featured_logo:
    type:    relative_resize
    options: { heighten: 78 }
dashboard_avatar:
    type:    relative_resize
    options: { widen: 170 }

Separate method and parameter keys there would have been superfluous. Only one method can specified at a time, and they each have a single parameter. On the subject of combining multiple operations, I think @lsmith told me that would be possible with a filter chain he had in this bundle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, after reading through this bundle's documentation, I still don't see any examples of configuring these filters in Twig. I just see them setup in YAML and then referred to in Twig by name. Can you point me towards an example?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, options come from the config, not from the Twig function. Missed this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're all good here? :)

The foreach syntax was just some shorthand to avoid calling key() and current). I'm definitely not using it for iteration.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, but this is quite confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lsmith77: What's your opinion here. Do you want me to use key() and current() or perhaps make the configuration more verbose so that method and parameter must be specified by their own keys?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm yeah .. i guess key() and current() would indeed be better. could you send another PR with that change .. but more importantly could you fix the README? i already added some docs there, but they are incomplete.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. I completely dropped the ball on porting over my README docs, sorry.

}
}
26 changes: 26 additions & 0 deletions Imagine/Filter/Loader/ResizeFilterLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace Liip\ImagineBundle\Imagine\Filter\Loader;

use Imagine\Filter\Basic\Resize;
use Imagine\Image\Box;

/**
* Loader for Imagine's basic resize filter.
*
* @author Jeremy Mikola <jmikola@gmail.com>
*/
class ResizeFilterLoader implements LoaderInterface
{
/**
* @see Liip\ImagineBundle\Imagine\Filter\Loader\LoaderInterface::load()
*/
public function load(ImageInterface $image, array $options = array())
{
list($width, $height) = $options['size'];

$filter = new Resize(new Box($width, $height));

return $filter->apply($image);
}
}
43 changes: 43 additions & 0 deletions Imagine/Filter/RelativeResize.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

namespace Liip\ImagineBundle\Imagine\Filter;

use Imagine\Exception\InvalidArgumentException;
use Imagine\Filter\FilterInterface;
use Imagine\Image\ImageInterface;
use Imagine\Image\BoxInterface;

/**
* Filter for resizing an image relative to its existing dimensions.
*
* @author Jeremy Mikola <jmikola@gmail.com>
*/
class RelativeResize implements FilterInterface
{
private $method;
private $parameter;

/**
* Constructs a RelativeResize filter with the given method and argument.
*
* @param string $method BoxInterface method
* @param mixed $parameter Parameter for BoxInterface method
*/
public function __construct($method, $parameter)
{
if (!in_array($method, array('heighten', 'increase', 'scale', 'widen'))) {
throw new InvalidArgumentException(sprintf('Unsupported method: ', $method));
}

$this->method = $method;
$this->parameter = $parameter;
}

/**
* @see Imagine\Filter\FilterInterface::apply()
*/
public function apply(ImageInterface $image)
{
return $image->resize(call_user_func(array($image->getSize(), $this->method), $this->parameter));
}
}
10 changes: 10 additions & 0 deletions Resources/config/imagine.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@

<!-- Filter loaders' classes -->

<parameter key="liip_imagine.filter.loader.relative_resize.class">Liip\ImagineBundle\Imagine\Filter\Loader\RelativeResizeFilterLoader</parameter>
<parameter key="liip_imagine.filter.loader.resize.class">Liip\ImagineBundle\Imagine\Filter\Loader\ResizeFilterLoader</parameter>
<parameter key="liip_imagine.filter.loader.thumbnail.class">Liip\ImagineBundle\Imagine\Filter\Loader\ThumbnailFilterLoader</parameter>

<!-- Data loaders' classes -->
Expand Down Expand Up @@ -109,6 +111,14 @@

<!-- Filter loaders -->

<service id="liip_imagine.filter.loader.relative_resize" class="%liip_imagine.filter.loader.relative_resize.class%">
<tag name="liip_imagine.filter.loader" loader="relative_resize" />
</service>

<service id="liip_imagine.filter.loader.resize" class="%liip_imagine.filter.loader.resize.class%">
<tag name="liip_imagine.filter.loader" loader="resize" />
</service>

<service id="liip_imagine.filter.loader.thumbnail" class="%liip_imagine.filter.loader.thumbnail.class%">
<tag name="liip_imagine.filter.loader" loader="thumbnail" />
</service>
Expand Down