[Webkit-unassigned] [Bug 90217] [chromium] Avoid calling getUniformLocation??() in the compositor startup

[bugzilla-daemon at webkit.org]

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


James Robinson <jamesr at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #150200|review-                     |
               Flag|                            |




--- Comment #5 from James Robinson <jamesr at chromium.org>  2012-06-29 12:26:07 PST ---
(From update of attachment 150200)
View in context: https://bugs.webkit.org/attachment.cgi?id=150200&action=review

If you update this patch to ToT it'll apply cleanly and trigger the auto-cc behavior, etc.  The EWS bubbles are purple since they couldn't apply this patch.

> Source/WebCore/platform/graphics/chromium/Extensions3DChromium.h:162
> +    void bindUniformLocationCHROMIUM(Platform3DObject program, GC3Dint location, const GC3Dchar* uniform);

I think your WebKit checkout is out of data - you don't have to modify Extensions3DChromium at all for compositor changes as of http://trac.webkit.org/changeset/121204.  Just adding the entry point to WebGraphicsContext3D is sufficient

> Source/WebCore/platform/graphics/chromium/ProgramBinding.cpp:64
>  void ProgramBindingBase::init(GraphicsContext3D* context, const String& vertexShader, const String& fragmentShader)

ProgramBinding is based on WebGraphicsContext3D, not GraphicsContext3D (cuts down on one level of indirection)

> Source/WebCore/platform/graphics/chromium/ProgramBinding.cpp:156
> +bool ProgramBindingBase::hasBindUniformsExtension(GraphicsContext3D* context) const

In the compositor we query all the extensions we may need up front (inside LayerRendererChromium::initialize) and then propagate them through as needed - this avoids any unnecessary get() calls and makes sure that all of our extensions use is centralized in one place.  All of the ProgramBinding uses and initialization calls are inside of LayerRendererChromium, so it should be pretty easy to plumb a bit for this through.

The extension name is bind_uniform (singular), so any functions/variables should stick to that and not plural "uniforms"

> Source/WebCore/platform/graphics/chromium/ShaderChromium.cpp:228
> +        "point[0]",

the example text in CHROMIUM_bind_uniform_location.txt queries the location of an array with just "point", not "point[0]". are both syntaxes allowed in getProgramUniform..() ?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.