Skip to content

Conversation

lashus
Copy link
Contributor

@lashus lashus commented May 18, 2019

I've changed the cropping behaviour as written in #709. Not sure though how to test it and also without using distort and SRT we are changing the image size (in GD that doesnt happen). Not sure how to refactor it other way though as with using image distortion with SRT it's hard to rotate it in same way (using same point).

@ausi
Copy link
Collaborator

ausi commented Jul 6, 2019

Not sure though how to test it

You can add a test like the following to the AbstractImageTest class:

public function testRotateWithCrop()
{
    $palette = new RGB();
    $color = $this
        ->getImagine()
        ->create(new Box(100, 100), $palette->color('#f00'))
        ->rotate(45, $palette->color('#fff'))
        ->crop(new Point(0, 0), new Box(100, 100))
        ->getColorAt(new Point(0, 50));
    $this->assertSame('#ffffff', (string) $color);
}

It creates a red 100x100 square image rotates it by 45 degrees and crops it to 100x100. After that the center pixel on the left side should be white as you can see in the following example images:

wrong: wrong correct: correct

we are changing the image size (in GD that doesnt happen)

I don’t understand what you mean by that. I tested all three drivers and (with the fix for Imagick) they all behave the same. GD also increases the image size if you rotate the image by 45 degrees.


Your pull request seems to include unrelated commits. I think you meant to include just the commit 58193ee correct?

@ausi
Copy link
Collaborator

ausi commented Jan 15, 2020

I created a followup pull request that includes the changes from my comments: #734

@mlocati
Copy link
Collaborator

mlocati commented Nov 3, 2020

Superseded by #734
Please don't try to fix multiple stuff in the same pull request

@mlocati mlocati closed this Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants