Bug 149144 – [Win] Tiled drawing is rendering more times than it should

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Brent Fulgham]

Errors in the Windows tiled drawing implementation cause WebKit to draw the same content multiple times. Furthermore, the nested CALayers used in the tiled drawing system caused the coordinate system to be flipped twice, causing some content to render upside down.

This patch corrects both of these problems. It also has the benefit of causing the debug border drawing for accelerated compositing to work properly on Windows.

[Brent Fulgham]

<rdar://problem/22313905>

[Brent Fulgham]

Created attachment 261166 [details]
WIP Patch (Not for review)

[Brent Fulgham]

Created attachment 261171 [details]
Patch v2

[Brent Fulgham]

Created attachment 261201 [details]
Patch

[Brent Fulgham]

Created attachment 261207 [details]
Patch (Correct deprecation warning)

[Simon Fraser (smfr)]

Comment on attachment 261207 [details]
Patch (Correct deprecation warning)

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

> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:-3038
> -    <ClCompile Include="..\dom\ClassNodeList.cpp">
> -      <Filter>dom</Filter>
> -    </ClCompile>

Why did this change?

> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:-3239
> -    <ClCompile Include="..\dom\NodeFilter.cpp">
> -      <Filter>dom</Filter>
> -    </ClCompile>

Ditto.

> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:74
> +#if OS(WINDOWS)
> +    if (platformCALayer->layerType() == PlatformCALayer::LayerTypeTiledBackingTileLayer) {
> +        // Tiled layers in Windows have flipped coordinates
> +        CGContextScaleCTM(context, 1, -1);
> +        CGContextTranslateCTM(context, 0, -indicatorBox.size.height);
> +    }
> +#endif

This doesn't seem like the right place for this code.

> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:95
>      CGContextSetTextMatrix(context, CGAffineTransformMakeScale(1, -1));
>      CGContextSelectFont(context, "Helvetica", 22, kCGEncodingMacRoman);

I think it would be better to move these lines into PlatformCALayer::drawTextAtPoint().

[Brent Fulgham]

Comment on attachment 261207 [details]
Patch (Correct deprecation warning)

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

>> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:-3038
>> -    </ClCompile>
> 
> Why did this change?

People update the WebCore.vcxproj file, but usually forget to update WebCore.vcxproj.filters. In this case, "ClassNodeList.cpp" was renamed to "dom\ClassCollection.cpp" in <http://trac.webkit.org/changeset/188735>, but the fact that this file was removed did not get registered until I updated the project file for this change.

>> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:-3239
>> -    </ClCompile>
> 
> Ditto.

Ditto. The "NodeFilter.cpp" file was deleted in <http://trac.webkit.org/changeset/189230>.

>> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:74
>> +#endif
> 
> This doesn't seem like the right place for this code.

Maybe, but this is called directly from TileGrid::platformCALayerPaintContents; I guess I could add the check there, and use the new "flipCoordinates" method at that point.

>> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:95
>>      CGContextSelectFont(context, "Helvetica", 22, kCGEncodingMacRoman);
> 
> I think it would be better to move these lines into PlatformCALayer::drawTextAtPoint().

OK!

[Brent Fulgham]

Comment on attachment 261207 [details]
Patch (Correct deprecation warning)

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

>>> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:95
>>>      CGContextSelectFont(context, "Helvetica", 22, kCGEncodingMacRoman);
>> 
>> I think it would be better to move these lines into PlatformCALayer::drawTextAtPoint().
> 
> OK!

Haha -- I have to! They are deprecated as well!

[Brent Fulgham]

Created attachment 261218 [details]
Patch v3 (revised for EWS and smfr's comments)

[Simon Fraser (smfr)]

Comment on attachment 261218 [details]
Patch v3 (revised for EWS and smfr's comments)

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

> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:-67
> +    CGContextSaveGState(context);
> +
>      indicatorBox.size.width = 12 + 10 * strlen(text);
>      indicatorBox.size.height = 27;
> -    CGContextSaveGState(context);

This change is a no-op.

[Brent Fulgham]

Comment on attachment 261218 [details]
Patch v3 (revised for EWS and smfr's comments)

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

>> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:-67
>> -    CGContextSaveGState(context);
> 
> This change is a no-op.

Whoops. I'll revert that before landing.

[Brent Fulgham]

Committed r189821: <http://trac.webkit.org/changeset/189821>