splittingred / phpThumbOf

A secure phpthumb output filter for MODx Revolution
http://rtfm.modx.com/display/addon/phpthumbof/
23 stars 17 forks source link

Portrait parameters overriding auto calculation. #35

Open AgentShark opened 12 years ago

AgentShark commented 12 years ago

If h is not set and hp is set, when iar=1, phpthumbof uses hp for ALL images. Should it not only use hp for portrait images? This behaviour happened before, but something broke it.

I want hp to be set for portrait images but leave all others alone to be auto calculated. This does not work as expected any more. [[... &w=600&hp=600&zc=1]]

oo12 commented 12 years ago

I think this is actually a bug with phpThumb (phpThumbOf is just a MODX wrapper for phpThumb). I ran into a similar problem and tracked it down to lines 2877-2891 in core/model/phpthumb/phpthumb.class.php

    if ($this->source_height > $this->source_width) {
        // portrait
        $this->w = phpthumb_functions::OneOfThese($this->wp, $this->w, $this->ws, $this->wl);
        $this->h = phpthumb_functions::OneOfThese($this->hp, $this->h, $this->hs, $this->hl);
    } elseif ($this->source_height < $this->source_width) {
        // landscape
        $this->w = phpthumb_functions::OneOfThese($this->wl, $this->w, $this->ws, $this->wp);
        $this->h = phpthumb_functions::OneOfThese($this->hl, $this->h, $this->hs, $this->hp);
    } else {
        // square
        $this->w = phpthumb_functions::OneOfThese($this->ws, $this->w, $this->wl, $this->wp);
        $this->h = phpthumb_functions::OneOfThese($this->hs, $this->h, $this->hl, $this->hp);
    }
    //$this->w = round($this->w ? $this->w : (($this->h && $this->source_height) ? $this->h * $this->source_width  / $this->source_height : $this->w));
    //$this->h = round($this->h ? $this->h : (($this->w && $this->source_width)  ? $this->w * $this->source_height / $this->source_width  : $this->h));

You can see what's going wrong. Since you don't have 'h' set it's just using 'hp' for height on a landscape image. I fixed it by changing that section of code to:

    if ($this->source_height > $this->source_width) {
        // portrait
        $this->w = phpthumb_functions::OneOfThese($this->wp, $this->w);
        $this->h = phpthumb_functions::OneOfThese($this->hp, $this->h);
    } elseif ($this->source_height < $this->source_width) {
        // landscape
        $this->w = phpthumb_functions::OneOfThese($this->wl, $this->w);
        $this->h = phpthumb_functions::OneOfThese($this->hl, $this->h);
    } else {
        // square
        $this->w = phpthumb_functions::OneOfThese($this->ws, $this->w);
        $this->h = phpthumb_functions::OneOfThese($this->hs, $this->h);
    }
    $this->w = round($this->w ? $this->w : (($this->h && $this->source_height) ? $this->h * $this->source_width / $this->source_height : $this->w));
    $this->h = round($this->h ? $this->h : (($this->w && $this->source_width)  ? $this->w * $this->source_height / $this->source_width  : $this->h));
    if ($this->w > $this->source_width) { //if new dimensions are larger than the original, revert
        $this->w = $this->source_width;
        $this->h = $this->source_height;
    }

I haven't tested this for other scenarios. It may break something else that I'm not aware of. But it did fix my problem, which was that I was trying to set either a height for landscape orientation or a width for portrait orientation, but wp=x&hl=y was effectively getting turned into w=x&h=y, which wasn't what I wanted.

tlasrich commented 9 years ago

+1 for the bugfix provided by oo12

vityakut commented 4 years ago

+1 oo12, thanks