[webkit-reviews] review denied: [Bug 92327] -webkit-background-clip:text is blurry in WebKit 1 apps when deviceScaleFactor > 1 : [Attachment 154524] Patch that should build everywhere

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Beth Dakin
<bdakin at apple.com>'s request for review:
Bug 92327: -webkit-background-clip:text is blurry in WebKit 1 apps when
deviceScaleFactor > 1
https://bugs.webkit.org/show_bug.cgi?id=92327

Attachment 154524: Patch that should build everywhere
https://bugs.webkit.org/attachment.cgi?id=154524&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=154524&action=review


> Source/WebCore/css/CSSGradientValue.cpp:78
> +    if (FrameView* frameView = renderer->view()->frameView()) {
> +	   if (HostWindow* hostWindow = frameView->hostWindow()) {
> +	       if (!hostWindow->ctmIncludesDeviceScaleFactor()) {
> +		   if (Page* page = renderer->document()->page())

Hmm, that's pretty icky. Could we just export a method on Page, perhaps, that
would make this cleaner?

> Source/WebCore/page/Chrome.h:90
> +	   virtual bool ctmIncludesDeviceScaleFactor();

This feels a bit too general. It doesn't say _which_ ctm. I could imagine that
we could easily have bugs where we use code to draw both into the tiles, and
into an ImageBuffer, and we need to be able to get both of them right.

I think 'device scale is baked into CTM' is something you have to ask the
GraphicsContext.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:836
> +	   if (FrameView* frameView = view()->frameView()) {
> +	       if (HostWindow* hostWindow = frameView->hostWindow()) {
> +		   if (!hostWindow->ctmIncludesDeviceScaleFactor()) {
> +		       if (Page* page = document()->page())
> +			   scaleFactorNotIncludedInCCTM =
page->deviceScaleFactor();

Similar ickitude.