Bug 75705 – [Chromium] Disable per-tile painting support on Windows.

• Component: WebKit Misc.
• Status: RESOLVED INVALID

[David Reveman]

Per-tile painting on WIN32 is not currently supported. Prevent it from being used and add ASSERT to reveal causes for it being enabled.

[David Reveman]

This is to fix http://code.google.com/p/chromium/issues/detail?id=107461 which still seem to be happening.

[David Reveman]

Created attachment 121436 [details]
Patch

[Adrienne Walker]

Isn't perTilePainting exposed in about:flags? Wouldn't that be the cause of it being turned on?

(That's really sad that per-tile painting is not supported on Windows.  Are there bugs open to address this? I was really hoping that we could get that to be the default texture uploader sometime soon.)

[David Reveman]

(In reply to comment #3)
> Isn't perTilePainting exposed in about:flags? Wouldn't that be the cause of it being turned on?

Oh, that's right. We're probably still seeing this crash because users explicitly enable it.

> 
> (That's really sad that per-tile painting is not supported on Windows.  Are there bugs open to address this? I was really hoping that we could get that to be the default texture uploader sometime soon.)

I haven't been able to determine the exact reason for why it's not working as don't have access to a Windows machine. I know that skia on Windows uses a HDC for text rendering and that breaks when using SkPictures. It still shouldn't crash the way it's currently doing.

[Adrienne Walker]

(In reply to comment #4)

> I haven't been able to determine the exact reason for why it's not working as don't have access to a Windows machine. I know that skia on Windows uses a HDC for text rendering and that breaks when using SkPictures. It still shouldn't crash the way it's currently doing.

If a bug isn't filed, can you file one?

[David Reveman]

(In reply to comment #5)
> (In reply to comment #4)
> 
> > I haven't been able to determine the exact reason for why it's not working as don't have access to a Windows machine. I know that skia on Windows uses a HDC for text rendering and that breaks when using SkPictures. It still shouldn't crash the way it's currently doing.
> 
> If a bug isn't filed, can you file one?

Done.

https://bugs.webkit.org/show_bug.cgi?id=75715

[James Robinson]

Please disable the about:flags entry on windows on the chrome side if it doesn't work. That might be sufficient for this issue.

[James Robinson]

Comment on attachment 121436 [details]
Patch

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

> Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:127
> +#if defined(WIN32)

in WebKit we use OS(WINDOWS)

> Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:128
> +    ASSERT(!host->settings().perTilePainting);

not sure this is useful to have. if we don't want to support the flag, then we should just ignore it

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp:99
> +#if defined(WIN32)
> +    ASSERT(!m_settings.perTilePainting); // No per-tile painting support on Windows.
> +#endif

how is this helpful? it won't help us with crashes in the field, since ASSERT()s are only active in debug builds

[David Reveman]

Created attachment 121464 [details]
Patch

[James Robinson]

Comment on attachment 121464 [details]
Patch

This looks good. It will make life a little harder for people trying to fix per-tile painting, so would you mind waiting to see if the about:flags entry gets the crashrate down low enough on canaries such that we don't need to do this before landing it?

R=me

[David Reveman]

(In reply to comment #10)
> (From update of attachment 121464 [details])
> This looks good. It will make life a little harder for people trying to fix per-tile painting, so would you mind waiting to see if the about:flags entry gets the crashrate down low enough on canaries such that we don't need to do this before landing it?
> 
> R=me

yup, I think that's a good idea.