[webkit-reviews] review granted: [Bug 172520] checkGPUStatus needs to exercise instancing calls : [Attachment 311040] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Dean Jackson <dino at apple.com>'s
request for review:
Bug 172520: checkGPUStatus needs to exercise instancing calls
https://bugs.webkit.org/show_bug.cgi?id=172520

Attachment 311040: Patch

https://bugs.webkit.org/attachment.cgi?id=311040&action=review




--- Comment #3 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 311040
  --> https://bugs.webkit.org/attachment.cgi?id=311040
Patch

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

Looks good! I mostly commented just to test my local WebKit build. :-) r=me.

> Source/WebCore/platform/graphics/GraphicsContext3D.h:1441
> +    unsigned m_statusCheckCount { 0 };

It seems like this could overflow if you status check counted enough times. But
maybe you reset later?

> Source/WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:65
> +static const unsigned statusCheckThreshold = 5;

I never know when we should be using constexpr. Here? No?

> Source/WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:652
> +    m_statusCheckCount = (m_statusCheckCount + 1) % statusCheckThreshold;

So this resets every statusCheckThreshold counts. Since that's currently set to
5, unsigned might be bigger than we need, but seems fine.