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
Status: RESOLVED FIXED
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
URL:
Keywords:
Depends on: 73317
Blocks:
  Show dependency tree / graph
 
Reported: 2011-12-18 23:36 PST by Alexandru Chiculita
Modified: 2011-12-22 05:26 PST (History)
6 users (show)

See Also:

Attachments
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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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.
https://bugs.webkit.org/show_bug.cgi?id=73317#c23
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.