Bug 84909 – Background tabs are fuzzy until repaint when deviceScaleFactor > 1

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Beth Dakin]

In WebKit2, if you open background tabs with a deviceScaleFactor > 1, they will appear fuzzy at first when you bring them to the foreground. A repaint fixes the problem.

<rdar://problem/11312064>

Patch forthcoming.

[Beth Dakin]

Created attachment 138903 [details]
Patch

[Darin Adler]

Comment on attachment 138903 [details]
Patch

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

> Source/WebKit2/Platform/cg/CGUtilities.h:32
> +void paintBitmapContext(CGContextRef, CGContextRef bitmapContext, CGPoint destination, CGRect source, float scaleFactor);

Why float instead of CGFloat?

> Source/WebKit2/UIProcess/mac/BackingStoreMac.mm:101
> +    IntSize scrolledRectSize = m_scrolledRect.size();
> +    scrolledRectSize.scale(m_deviceScaleFactor);

I think it’s a little unclear to have a local variable named scrolledRectSize that is not the same as m_scrolledRect.size(). Maybe its should be named scrolledRectPixelsSize or scaledSize or something like that?

> Source/WebKit2/UIProcess/mac/BackingStoreMac.mm:109
> -    CGContextTranslateCTM(context.get(), 0, m_scrolledRect.size().height());
> +    CGContextTranslateCTM(context.get(), 0, m_scrolledRect.height());

This change seems to be unneeded. I am guessing you changed this to use scrolledRectSize and then later changed it back.

[Beth Dakin]

(In reply to comment #2)
> (From update of attachment 138903 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138903&action=review
> 
> > Source/WebKit2/Platform/cg/CGUtilities.h:32
> > +void paintBitmapContext(CGContextRef, CGContextRef bitmapContext, CGPoint destination, CGRect source, float scaleFactor);
> 
> Why float instead of CGFloat?
> 

No good reason. Will fix.

> > Source/WebKit2/UIProcess/mac/BackingStoreMac.mm:101
> > +    IntSize scrolledRectSize = m_scrolledRect.size();
> > +    scrolledRectSize.scale(m_deviceScaleFactor);
> 
> I think it’s a little unclear to have a local variable named scrolledRectSize that is not the same as m_scrolledRect.size(). Maybe its should be named scrolledRectPixelsSize or scaledSize or something like that?
> 

Will change.

> > Source/WebKit2/UIProcess/mac/BackingStoreMac.mm:109
> > -    CGContextTranslateCTM(context.get(), 0, m_scrolledRect.size().height());
> > +    CGContextTranslateCTM(context.get(), 0, m_scrolledRect.height());
> 
> This change seems to be unneeded. I am guessing you changed this to use scrolledRectSize and then later changed it back.

Good catch. Will fix.

Thanks!

[Beth Dakin]

Committed change: http://trac.webkit.org/changeset/115266

[Darin Adler]

Comment on attachment 138903 [details]
Patch

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

>>> Source/WebKit2/Platform/cg/CGUtilities.h:32
>>>  void paintImage(CGContextRef, CGImageRef, CGFloat scaleFactor, CGPoint destination, CGRect source);
>>> -void paintBitmapContext(CGContextRef, CGContextRef bitmapContext, CGPoint destination, CGRect source);
>>> +void paintBitmapContext(CGContextRef, CGContextRef bitmapContext, CGPoint destination, CGRect source, float scaleFactor);
>> 
>> Why float instead of CGFloat?
> 
> No good reason. Will fix.

On further reflection, it might make more sense to keep the argument order the same for these two functions. As a follow-up cleanup patch we could move the scale factor back before the destination rect to match paintImage.

[Beth Dakin]

(In reply to comment #5)
> On further reflection, it might make more sense to keep the argument order the same for these two functions. As a follow-up cleanup patch we could move the scale factor back before the destination rect to match paintImage.

Done: http://trac.webkit.org/changeset/115270