[webkit-reviews] review denied: [Bug 98977] Shader translator needs option to clamp uniform array accesses in vertex shaders : [Attachment 169062] Patch

[bugzilla-daemon at webkit.org]

Alok Priyadarshi <alokp at chromium.org> has denied Dean Jackson
<dino at apple.com>'s request for review:
Bug 98977: Shader translator needs option to clamp uniform array accesses in
vertex shaders
https://bugs.webkit.org/show_bug.cgi?id=98977

Attachment 169062: Patch
https://bugs.webkit.org/attachment.cgi?id=169062&action=review

------- Additional Comments from Alok Priyadarshi <alokp at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=169062&action=review


TranslatorHLSL have a similar problem. You should look at it for inspiration on
how they separate writing out a header and body. I would do it in a different
way:

class TOutputGLSL {
  TOutputGLSL(Spec, bool clampArrayIndex);

  void output(TInfoSinkBase& sink) {
    TOutputXXX traverser(clamp);
    root->traverse(&traverser);
    sink << traverser.header();
    sink << traverser.body();
  }
};

If you want I can take submit a patch to ANGLE.

> Source/ThirdParty/ANGLE/src/compiler/OutputGLSLBase.cpp:69
> +TOutputGLSLBase::TOutputGLSLBase(TInfoSinkBase& objSink, ArrayBoundsClamper&
arrayBoundsClamper)

Adding a dependency on ArrayBoundsClamper does not seem right.

> Source/ThirdParty/ANGLE/src/compiler/TranslatorGLSL.cpp:49
> +    // Write array bounds clamping definition if necessary.

duplicated code from TranslatorESSL.