Bug 93558 - [chromium] Pass mask scale and offset to shaders for correct masking
Summary: [chromium] Pass mask scale and offset to shaders for correct masking
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: Shawn Singh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-08 17:15 PDT by Shawn Singh
Modified: 2012-08-09 12:39 PDT (History)
6 users (show)

See Also:


Attachments
Patch (15.32 KB, patch)
2012-08-08 17:18 PDT, Shawn Singh
no flags Details | Formatted Diff | Diff
Added layout test and fixed errors (25.89 KB, patch)
2012-08-09 11:49 PDT, Shawn Singh
enne: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shawn Singh 2012-08-08 17:15:01 PDT
In the current code:
(1) masks force us to create a renderSurface for that layer
(2) masks are drawn 1:1 with respect to the renderSurface, but they should be drawn 1:1 with respect to the layer that owns the surface.

usually this isn't a problem, but if the renderSurface is clipped by something else (for example, the viewport), this causes the renderSurface to resize to the clipped area, and the mask incorrectly resizes along with the surface.

The solution we have chosen at this time is to pass the scale/offset data that the mask needs to correctly compute its texture coordinates in the fragment shader.
Comment 1 Shawn Singh 2012-08-08 17:18:38 PDT
Created attachment 157345 [details]
Patch
Comment 2 WebKit Review Bot 2012-08-08 17:20:51 PDT
Attachment 157345 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/platform/graphics/chromium/..." exit_code: 1
Source/WebCore/platform/graphics/chromium/ShaderChromium.cpp:697:  v_maskTexCoord is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/platform/graphics/chromium/ShaderChromium.cpp:750:  v_maskTexCoord is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Shawn Singh 2012-08-08 17:22:30 PDT
Few points to make:

(1) the style checker will complain, but this seems to fit the style we had for written shaders already.

(2) I will be writing layout tests for this, but it's ready for review otherwise.  I suspect you guys may have something to say about how we could group these new values into fewer args that get passed around. =)
Comment 4 Shawn Singh 2012-08-08 17:32:38 PDT
Comment on attachment 157345 [details]
Patch

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

>> Source/WebCore/platform/graphics/chromium/ShaderChromium.cpp:750
>> +            vec2 v_maskTexCoord = vec2(maskTexCoordOffset.x + v_texCoord.x * maskTexCoordScale.x, maskTexCoordOffset.y + v_texCoord.y * maskTexCoordScale.y);
> 
> v_maskTexCoord is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]

On second thought, I shouldn't be prefixing this with v_ anyway, so this won't be an issue on the next patch.
Comment 5 WebKit Review Bot 2012-08-08 17:43:32 PDT
Comment on attachment 157345 [details]
Patch

Attachment 157345 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13452834
Comment 6 Shawn Singh 2012-08-09 11:49:43 PDT
Created attachment 157504 [details]
Added layout test and fixed errors
Comment 7 Adrienne Walker 2012-08-09 12:28:27 PDT
Comment on attachment 157504 [details]
Added layout test and fixed errors

R=me.  Nice layout test!  Please rebaseline this after landing.
Comment 8 Shawn Singh 2012-08-09 12:39:47 PDT
Committed r125193: <http://trac.webkit.org/changeset/125193>