Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Conversation

@DHowett-MSFT
Copy link

  • -[CALayer renderInContext:] has the ability to render a cached version of itself; it was doing so upside-down because it was using a Cairo implementation detail.
  • -[UIImage drawAtPoint:] squashed every image it rendered because it switched width & height.

The bug in UIImage dates back to the Cairo implementation as well; the invalid size was passed to a function that ignored it. When our refactor made use of that parameter, things rapidly went south.
We did not catch the second regression because all the images we drew with it were square.

pos.size.width = (img_height / _scale);
pos.size.height = (img_width / _scale);
pos.size.height = (img_height / _scale);
pos.size.width = (img_width / _scale);
Copy link
Author

Choose a reason for hiding this comment

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

@aballway might like this

Copy link
Contributor

Choose a reason for hiding this comment

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

typical think too fast or type too fast.


In reply to: 103579177 [](ancestors = 103579177)

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: position is a very silly name for a rect 😝

Copy link
Author

Choose a reason for hiding this comment

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

nit: oh well :)

// |0 -1 0|
// |0 h 1|
CGContextSaveGState(ctx);
CGContextConcatCTM(ctx, CGAffineTransformMake(1, 0, 0, -1, 0, destRect.size.height));
Copy link
Contributor

Choose a reason for hiding this comment

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

CGAffineTransformMake [](start = 32, length = 21)

mind adding a comment calling out why this this transform is needed so that other people without the context can understand the reasoning behind. Thx!

Copy link
Author

Choose a reason for hiding this comment

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

@yiyang-msft
Copy link
Contributor

:shipit:

1 similar comment
@oliversa-msft
Copy link
Contributor

:shipit:

pos.size.width = (img_height / _scale);
pos.size.height = (img_width / _scale);
pos.size.height = (img_height / _scale);
pos.size.width = (img_width / _scale);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: position is a very silly name for a rect 😝

// |1 0 0| is the transformation matrix for flipping a rect anchored at 0,0 about its Y midpoint.
// |0 -1 0|
// |0 h 1|
CGContextSaveGState(ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

is saving/popping g-state relatively free? we should be able to get away with just reverting the CTM if not.

Copy link
Author

Choose a reason for hiding this comment

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

It's very cheap.

@DHowett-MSFT
Copy link
Author

This is in CI build; git-lfs is being particularly slow today.

@DHowett-MSFT
Copy link
Author

@rajsesh-msft if you end up being the one to merge this (I'll try to monitor the build status!), would you mind copying the Pull Request body into the squash message body?

Thanks :)

@DHowett-MSFT DHowett-MSFT merged commit 7fd00a1 into microsoft:develop Mar 1, 2017
@DHowett-MSFT DHowett-MSFT deleted the 201702-fix-2095 branch March 1, 2017 04:11
rajsesh pushed a commit to rajsesh/WinObjC that referenced this pull request Mar 1, 2017
…#2117)

* `-[CALayer renderInContext:]` has the ability to render a cached version of itself; it was doing so upside-down because it was using a Cairo implementation detail.
* `-[UIImage drawAtPoint:]` squashed every image it rendered because it switched width & height.

The bug in UIImage dates back to the Cairo implementation as well; the invalid size was passed to a function that ignored it. When our refactor made use of that parameter, things rapidly went south.
We did not catch the second regression because all the images we drew with it were square.

Fixes microsoft#2095.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants