Bug 74840 - [CSS Shaders] Follow up bug to fix issues mentioned in comment 23 from bug 73317
Summary: [CSS Shaders] Follow up bug to fix issues mentioned in comment 23 from bug 73317
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexandru Chiculita
Depends on: 73317
  Show dependency treegraph
Reported: 2011-12-18 23:36 PST by Alexandru Chiculita
Modified: 2011-12-22 05:26 PST (History)
6 users (show)

See Also:

Patch V1 (57.23 KB, patch)
2011-12-20 05:35 PST, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V2 (56.68 KB, patch)
2011-12-20 06:05 PST, Alexandru Chiculita
zimmermann: review+
zimmermann: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandru Chiculita 2011-12-18 23:36:00 PST
There are some issues mentioned by Nikolas Zimmermann after the patch got reviewed and committed. This bug is a follow up that fixes those issues.
Comment 1 Alexandru Chiculita 2011-12-20 05:35:43 PST
Created attachment 120014 [details]
Patch V1

The spec seems to have been updated to use 1 by 1 as the default mesh. I've also updated that and added some comments.
Comment 2 Alexandru Chiculita 2011-12-20 06:05:10 PST
Created attachment 120018 [details]
Patch V2

Using the StringBuilder only inside the CachedShader.cpp file.
Comment 3 Nikolas Zimmermann 2011-12-21 12:25:17 PST
Comment on attachment 120018 [details]
Patch V2

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

Much nicer, thanks a lot. r=me with a few comments:

> Source/WebCore/platform/graphics/filters/CustomFilterShader.cpp:51
> +const char* CustomFilterShader::defaultFragmentShaderString =
> +        "precision mediump float;\n" \

I admit, I like the other solution much more now :-)
It's much more readable, would you mind updating it once again?

> Source/WebCore/platform/graphics/filters/CustomFilterShader.h:79
> +    static const char* defaultVertexShaderString;
> +    static const char* defaultFragmentShaderString;

This could be removed again using this SHADER technique.
Comment 4 Alexandru Chiculita 2011-12-22 05:26:43 PST
Landed in r103535. Closing the bug.