[webkit-reviews] review granted: [Bug 187750] Safari WebGL does not consistently provide correct GPU context on eGPU systems : [Attachment 345385] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Justin Fan <justin_fan at apple.com>'s
request for review:
Bug 187750: Safari WebGL does not consistently provide correct GPU context on
eGPU systems
https://bugs.webkit.org/show_bug.cgi?id=187750

Attachment 345385: Patch

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




--- Comment #23 from Dean Jackson <dino at apple.com> ---
Comment on attachment 345385
  --> https://bugs.webkit.org/attachment.cgi?id=345385
Patch

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

> Source/WebCore/platform/graphics/GraphicsContext3DManager.cpp:167
> +    ASSERT(!m_contexts.contains(contextWithWindow));

We should probably also ASSERT on the unlikely case that { context, nullptr }
is present in m_contexts.

> Source/WebCore/platform/graphics/GraphicsContext3DManager.cpp:175
> +    ContextWithWindow contextWithWindow { context, nullptr };
> +    ASSERT(m_contexts.contains(contextWithWindow));
> +    m_contexts.removeFirst(contextWithWindow);

What am I misunderstanding here? You inserted the context with it's HostWindow
pointer. But you're removing it with a nullptr. How is it being found?

(LATER) - oh, I see, it's got an operator==

In this case it might be better to use a HashMap of <GC3D*, HostWindow*>. You
can still iterate over all the elements easily.