Bug 136734 - Check for varying packing restrictions per program instead of per shader
Summary: Check for varying packing restrictions per program instead of per shader
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-09-10 21:39 PDT by Roger Fong
Modified: 2014-09-11 12:24 PDT (History)
9 users (show)

See Also:


Attachments
patch (10.67 KB, patch)
2014-09-10 21:50 PDT, Roger Fong
no flags Details | Formatted Diff | Diff
patch (10.67 KB, patch)
2014-09-11 10:13 PDT, Roger Fong
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roger Fong 2014-09-10 21:39:58 PDT
rdar://problem/16308409
Comment 1 Roger Fong 2014-09-10 21:50:43 PDT
Created attachment 237931 [details]
patch
Comment 2 WebKit Commit Bot 2014-09-10 21:52:51 PDT
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Comment 3 Roger Fong 2014-09-10 21:56:43 PDT
The spec says:

If the packing algorithm fails either for the uniform variables of a shader or for the varying variables of a program, compilation or linking must fail.

I originally understood compilation to mean, the shaders fail to compile (since you don't compile a program, you just link it). But in this case, I think compilation means the same thing as linking.
Comment 4 Roger Fong 2014-09-11 10:13:58 PDT
Created attachment 237961 [details]
patch

with build fix
Comment 5 Dean Jackson 2014-09-11 11:50:18 PDT
Comment on attachment 237961 [details]
patch

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

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:368
> +        const char* builtIns[] = {
> +            "gl_FragCoord",
> +            "gl_FrontFacing",
> +            "gl_PointCoord"
> +        };
> +
> +        bool isBuiltIn = false;
> +        for (int i = 0; i < 3; ++i) {
> +            if (symbolName == builtIns[i]) {
> +                isBuiltIn = true;
> +                break;
> +            }
> +        }
> +
> +        if (isBuiltIn)
> +            continue;

I don't really have a preference here, but maybe it might be simpler to do if (symbolName == "gl_FragCoord" || symbolName ==...) continue;
Comment 6 Roger Fong 2014-09-11 12:24:26 PDT
Took Dean's suggestion.

Committed:
http://trac.webkit.org/changeset/173527