Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(5)

Issue 2828353003: Determine mask UVs based on texture size (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 4 weeks ago by ericrk
Modified:
3 months, 2 weeks ago
Reviewers:
sunxd, enne (OOO)
CC:
cc-bugs_chromium.org, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Determine mask UVs based on texture size Currently, mask SINGLE_TEXTURE_MASK UVs are determined solely by the size of the masked layer. This means that if the mask texture size does not match the expected texture size, we will end up squishing the mask when it's applied. This CL adds a new out param, |resource_uv_size| to GetContentsResourceId. This param represents the UV coordinate size of the mask contents within its texture. This factor is used to adjust the existing calculation, allowing for non-exact mask texture sizes. R=sunxd@chromium.org, enne@chromium.org BUG=713872 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2828353003 Cr-Commit-Position: refs/heads/master@{#469464} Committed: https://chromium.googlesource.com/chromium/src/+/664405a74320db4031a18694358f7d4fef327662

Patch Set 1 #

Total comments: 1

Patch Set 2 : Add tolerance to filter_effects.html #

Patch Set 3 : Fix scaling issues #

Patch Set 4 : comment tweaks #

Patch Set 5 : fix build #

Patch Set 6 : small fix #

Total comments: 8

Patch Set 7 : Feedback #

Total comments: 3

Patch Set 8 : nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+217 lines, -28 lines) Patch
M cc/layers/layer_impl.h View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M cc/layers/layer_impl.cc View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M cc/layers/picture_layer_impl.h View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M cc/layers/picture_layer_impl.cc View 1 2 3 4 5 6 2 chunks +16 lines, -2 lines 0 comments Download
M cc/layers/picture_layer_impl_unittest.cc View 1 2 3 4 5 6 5 chunks +14 lines, -6 lines 0 comments Download
M cc/layers/render_surface_impl.cc View 1 2 3 4 5 6 1 chunk +9 lines, -6 lines 0 comments Download
M cc/layers/render_surface_impl_unittest.cc View 1 2 3 4 5 6 2 chunks +9 lines, -5 lines 0 comments Download
A cc/test/data/mask_of_larger_layer.png View 1 2 3 4 5 6 Binary file 0 comments Download
A cc/test/data/mask_with_non_exact_texture_size.png View 1 2 3 4 5 6 Binary file 0 comments Download
M cc/test/fake_mask_layer_impl.h View 1 2 1 chunk +7 lines, -1 line 0 comments Download
M cc/test/fake_mask_layer_impl.cc View 1 2 1 chunk +4 lines, -1 line 0 comments Download
M cc/test/fake_picture_layer.h View 1 2 2 chunks +5 lines, -0 lines 0 comments Download
M cc/test/fake_picture_layer.cc View 1 2 3 4 1 chunk +13 lines, -4 lines 0 comments Download
M cc/trees/layer_tree_host_pixeltest_masks.cc View 1 2 3 4 5 6 7 2 chunks +58 lines, -0 lines 0 comments Download
M cc/trees/layer_tree_host_unittest.cc View 1 2 3 4 5 6 1 chunk +76 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Dependent Patchsets:

Messages

Total messages: 46 (31 generated)
ericrk
This is the precursor to allowing CC tiles to re-use larger resources for raster.
3 months, 4 weeks ago (2017-04-20 21:52:22 UTC) #4
enne (OOO)
lgtm Thanks for all the investigation. https://codereview.chromium.org/2828353003/diff/1/cc/trees/layer_tree_host_pixeltest_masks.cc File cc/trees/layer_tree_host_pixeltest_masks.cc (right): https://codereview.chromium.org/2828353003/diff/1/cc/trees/layer_tree_host_pixeltest_masks.cc#newcode151 cc/trees/layer_tree_host_pixeltest_masks.cc:151: TEST_P(LayerTreeHostMasksPixelTest, MaskOfLargerLayer) { ...
3 months, 4 weeks ago (2017-04-20 22:44:08 UTC) #8
sunxd
On 2017/04/20 22:44:08, enne wrote: > lgtm > > Thanks for all the investigation. > ...
3 months, 4 weeks ago (2017-04-21 16:08:07 UTC) #9
ericrk
Found some issues with compositor transforms in my last approach. I think there's a way ...
3 months, 2 weeks ago (2017-05-02 18:27:27 UTC) #23
enne (OOO)
The new pixel test here is just verifying the behavior when the mask is a ...
3 months, 2 weeks ago (2017-05-02 21:31:38 UTC) #28
sunxd
https://codereview.chromium.org/2828353003/diff/120001/cc/trees/layer_tree_host_pixeltest_masks.cc File cc/trees/layer_tree_host_pixeltest_masks.cc (right): https://codereview.chromium.org/2828353003/diff/120001/cc/trees/layer_tree_host_pixeltest_masks.cc#newcode164 cc/trees/layer_tree_host_pixeltest_masks.cc:164: mask->SetLayerMaskType(Layer::LayerMaskType::SINGLE_TEXTURE_MASK); I made a recent change that makes pixeltest ...
3 months, 2 weeks ago (2017-05-03 15:14:13 UTC) #31
ericrk
Updated. Thanks! https://codereview.chromium.org/2828353003/diff/120001/cc/layers/picture_layer_impl.cc File cc/layers/picture_layer_impl.cc (right): https://codereview.chromium.org/2828353003/diff/120001/cc/layers/picture_layer_impl.cc#newcode881 cc/layers/picture_layer_impl.cc:881: // coordinatesis are (1.0f, 1.0f). However, if ...
3 months, 2 weeks ago (2017-05-04 19:23:53 UTC) #32
enne (OOO)
lgtm, thanks for the extra test :)
3 months, 2 weeks ago (2017-05-04 19:40:48 UTC) #35
sunxd
On 2017/05/04 19:40:48, enne wrote: > lgtm, thanks for the extra test :) lgtm % ...
3 months, 2 weeks ago (2017-05-04 19:43:33 UTC) #36
sunxd
https://codereview.chromium.org/2828353003/diff/140001/cc/trees/layer_tree_host_pixeltest_masks.cc File cc/trees/layer_tree_host_pixeltest_masks.cc (right): https://codereview.chromium.org/2828353003/diff/140001/cc/trees/layer_tree_host_pixeltest_masks.cc#newcode205 cc/trees/layer_tree_host_pixeltest_masks.cc:205: mask->SetLayerMaskType(Layer::LayerMaskType::SINGLE_TEXTURE_MASK); Nit: Could you also change this to mask_type_? ...
3 months, 2 weeks ago (2017-05-04 19:43:49 UTC) #37
ericrk
https://codereview.chromium.org/2828353003/diff/140001/cc/trees/layer_tree_host_pixeltest_masks.cc File cc/trees/layer_tree_host_pixeltest_masks.cc (right): https://codereview.chromium.org/2828353003/diff/140001/cc/trees/layer_tree_host_pixeltest_masks.cc#newcode205 cc/trees/layer_tree_host_pixeltest_masks.cc:205: mask->SetLayerMaskType(Layer::LayerMaskType::SINGLE_TEXTURE_MASK); On 2017/05/04 19:43:49, sunxd wrote: > Nit: Could ...
3 months, 2 weeks ago (2017-05-04 19:51:38 UTC) #38
ericrk
https://codereview.chromium.org/2828353003/diff/140001/cc/trees/layer_tree_host_pixeltest_masks.cc File cc/trees/layer_tree_host_pixeltest_masks.cc (right): https://codereview.chromium.org/2828353003/diff/140001/cc/trees/layer_tree_host_pixeltest_masks.cc#newcode205 cc/trees/layer_tree_host_pixeltest_masks.cc:205: mask->SetLayerMaskType(Layer::LayerMaskType::SINGLE_TEXTURE_MASK); On 2017/05/04 19:51:38, ericrk wrote: > On 2017/05/04 ...
3 months, 2 weeks ago (2017-05-04 19:55:06 UTC) #39
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2828353003/160001
3 months, 2 weeks ago (2017-05-04 19:55:50 UTC) #42
sunxd
On 2017/05/04 19:55:06, ericrk wrote: > https://codereview.chromium.org/2828353003/diff/140001/cc/trees/layer_tree_host_pixeltest_masks.cc > File cc/trees/layer_tree_host_pixeltest_masks.cc (right): > > https://codereview.chromium.org/2828353003/diff/140001/cc/trees/layer_tree_host_pixeltest_masks.cc#newcode205 > ...
3 months, 2 weeks ago (2017-05-04 20:31:55 UTC) #43
commit-bot: I haz the power
3 months, 2 weeks ago (2017-05-04 20:58:53 UTC) #46
Message was sent while issue was closed.
Committed patchset #8 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/664405a74320db4031a18694358f...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b