Yahoo.com has fragments of wrongly-scaled content throughout when using CGDisplayListImageBufferBackend

[Tim Horton]

Yahoo.com has fragments of wrongly-scaled content throughout when using CGDisplayListImageBufferBackend

[Tim Horton]

Created attachment 442753 [details]
Patch

[Tim Horton]

Created attachment 442754 [details]
Patch

[Darin Adler]

Comment on attachment 442754 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=442754&action=review

> Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.cpp:40
> +    GraphicsContextCGDisplayList(CGContextRef cgContext, float immutableBaseScaleFactor)

Since AffineTransform is based on double, maybe we should store this as a double? Generally speaking not sure how performance critical any of this is, but written like this we have a float to double conversion every time setCTM and getCTM are called. But we also have floating point division in setCTM, and making the same AffineTransform each time the function is called, so I suspect it’s likely not all that performance critical.

[Tim Horton]

(In reply to Darin Adler from comment #3)
> Comment on attachment 442754 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=442754&action=review
> 
> > Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.cpp:40
> > +    GraphicsContextCGDisplayList(CGContextRef cgContext, float immutableBaseScaleFactor)
> 
> Since AffineTransform is based on double, maybe we should store this as a
> double?

True! I was following all of our deviceScaleFactors, which are float-y, since that's where it *comes* from.

> Generally speaking not sure how performance critical any of this is,
> but written like this we have a float to double conversion every time setCTM
> and getCTM are called. But we also have floating point division in setCTM,
> and making the same AffineTransform each time the function is called, so I
> suspect it’s likely not all that performance critical.

Neither are /very/ hot in my testing, no, but these are both good points. I will adjust slightly to address both :) (in fact, because CGDisplayListImageBufferBackend's context is only ever used as the secondary context in a BifurcatedGraphicsContext, its getCTM is actually never called, I only included it here for completeness's sake and in case we ever want to use it directly).

[Tim Horton]

Created attachment 442774 [details]
Patch

[EWS]

Committed r285017 (243661@main): <https://commits.webkit.org/243661@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442774 [details].

[Radar WebKit Bug Importer]

<rdar://problem/84792074>