Bug 13578 – When QD plugins draw to an offscreen bitmap the origin is not correct

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Bruce Q. Hammond]

The CGContextRef's origin  is ignored when the offscreen GWorld is constructed. 
The origin is implied by the CGContextRef's CGAffineTransform.

[Bruce Q. Hammond]

Created attachment 14323 [details]
QD plugins now respect the origin of CGBitmapContext

[Darin Adler]

Comment on attachment 14323 [details]
QD plugins now respect the origin of CGBitmapContext 

Looks fine. But why not use CGPointApplyAffineTransform instead of doing explicit multiplication?

[David Harrison]

rdar://5182103

[Timothy Hatcher]

Comment on attachment 14323 [details]
QD plugins now respect the origin of CGBitmapContext 

(int32)-boundsInWindow.origin.y - (offscreenBounds.bottom + offscreenMatrix.ty * offscreenMatrix.d)

Please explain why you need to multiply offscreenMatrix.ty by offscreenMatrix.d but not do something similar for offscreenMatrix.tx. Please explain this code better in the comments.

Can you also use CGPointApplyAffineTransform like Darin asked?

[Darin Adler]

Comment on attachment 14323 [details]
QD plugins now respect the origin of CGBitmapContext 

Someone told me that Bruce is working on a version of this that uses CGPointApplyAffineTransform, so review- until we have that one.

[Bruce Q. Hammond]

Created attachment 14404 [details]
respect the origin implied by the bitmap contexts affine tranformation matrix

Implements the changes requested by reviewers to go though CGPointApplyAffineTransform to create the origin data.  Personally I don't like this patch as much as the previous one because it requires yet another call to CGPointApplyAffineTransform in order to determine if the axes are flipped.  However it is simply a mater of taste and is functionally identical;  it may be more clear to some maintainers what is happening.
I also now treat Y-Axis and and X-Axis identically even though inverting the X-Axis would be unusual.

[Darin Adler]

Comment on attachment 14404 [details]
respect the origin implied by the bitmap contexts affine tranformation matrix

r=me, assuming that you (Bruce) have a test case that shows this works well, since Safari doesn't use this code path.

[Darin Adler]

(In reply to comment #7)
> r=me, assuming that you (Bruce) have a test case that shows this works well,
> since Safari doesn't use this code path.

Bruce confirmed that he does have a test, so confirmed: r=me.

[Mark Rowe (bdash)]

Landed in r21322.

[Mark Rowe (bdash)]

A build fix in r21323 was needed to keep things building with the ancient version of Xcode the buildslaves are currently using.

[Bruce Q. Hammond]

Created attachment 14573 [details]
Fixes the sign of the Y-Origin adjustment

The Y-origin adjustment was backwards on the previous patch.  This fixes it.

[Mark Rowe (bdash)]

Comment on attachment 14573 [details]
Fixes the sign of the Y-Origin adjustment

It'd be *really* handy if all of these changes had some form of regression testing.  There have been several issues with past iterations of related patches, and without any way to test these (Safari doesn't touch this code path) it's impossible to know if and when this codepath breaks in the future.