[Webkit-unassigned] [Bug 58003] chromium support for glSetLatch and glWaitLatch between 3D contexts
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Apr 11 16:38:20 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=58003
--- Comment #7 from John Bates <jbates at google.com> 2011-04-11 16:38:20 PST ---
(From update of attachment 88914)
View in context: https://bugs.webkit.org/attachment.cgi?id=88914&action=review
>> Source/WebCore/ChangeLog:3
>> + Reviewed by Ken Russell.
>
> Leave this line as "Reviewed by NOBODY (OOPS!)". The commit scripts will rewrite it after it's been reviewed.
done
>> Source/WebCore/platform/graphics/chromium/Extensions3DChromium.h:73
>> + void setLatchCHROMIUM(GC3Dint shmId, GC3Duint latchId);
>
> Per offline discussion, let's remove the shmId argument now.
done
>> Source/WebCore/platform/graphics/chromium/GLES2Canvas.cpp:177
>> + m_a(a), m_b(b), m_c(c), m_d(d)
>
> This whitespace-only change doesn't belong in this patch.
done
>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:276
>> + // Before drawLayers:
>
> This comment seems useless.
It's meant to correlate the code with drawLayers, so that people aren't inclined to move it up or down, etc.
>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:277
>> + if (hardwareCompositing()) {
>
> Even though your current code runs, for correctness, the following block of code needs to be run against the layer renderer's GraphicsContext3D and all of the child layers' contexts:
>
> if (m_context->getExtensions()->supports("GL_CHROMIUM_latch"))
> m_context->getExtensions()->ensureEnabled("GL_CHROMIUM_latch");
>
> and the code which uses getChildToParentLatchCHROMIUM and waitLatchCHROMIUM needs to be guarded by a test:
>
> if (extensions->supports("GL_CHROMIUM_latch"))
>
> See e.g. Source/WebCore/platform/graphics/gpu/DrawingBuffer.cpp.
done
>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:278
>> + // The multithreaded compositor case does not work as long as
>
> Change this to "FIXME: ... case will not work".
done
>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:281
>> + // potentially clobbering the parent texture that is beibng renderered
>
> typo: beibng
done
>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:353
>> + // Before updateCompositorResourcesRecursive, when the compositor runs in
>
> Please add a FIXME here.
done
>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:967
>> +
>
> This whitespace-only change doesn't belong in this patch. I suggest you try to configure your editor.
done
>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:1125
>> +// TODO(jbates): when compositor is multithreaded and copyTexImage2D bug is fixed,
>
> WebKit uses FIXME rather than TODO().
done
>> Source/WebKit/chromium/ChangeLog:3
>> + Reviewed by Ken Russell.
>
> Per above, leave the OOPS line.
done
>> Source/WebKit/chromium/public/WebGraphicsContext3D.h:175
>> + virtual void setLatchCHROMIUM(WGC3Dint shmId, WGC3Duint latchId) {} // TODO = 0;
>
> TODO -> FIXME:
>
> Also, per offline discussion, let's remove the shmId argument, as it's being removed from the implementation.
done
>> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:254
>> +
>
> Whitespace-only change.
done
>> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:806
>> +DELEGATE_TO_IMPL_2(setLatchCHROMIUM, GC3Dint, GC3Duint)
>
> Per offline discussion, remove first argument.
done
>> Source/WebKit/chromium/src/GraphicsContext3DInternal.h:277
>> + void setLatchCHROMIUM(GC3Dint shmId, GC3Duint latchId);
>
> Remove shmId argument.
done
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list