Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1889153002:
cc: nine patch: add occlusion support (Closed)
|
Created:
4 years, 8 months ago by llandwerlin-old Modified:
4 years, 8 months ago CC:
cc-bugs_chromium.org, chromium-reviews, danakj+watch_chromium.org, jbauman+watch_chromium.org, kalyank, msw, piman+watch_chromium.org, sadrul, sievers+watch_chromium.org, Ian Vollick Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptioncc: nine patch: add occlusion support
When using a NinePatchLayer to draw shadows on windows on ChromeOS part of
patches which are completely transparent are drawn on top of the window.
This prevents for example videos to be promoted into layers.
This patch introduce an optional property on the NinePatchLayer :
layer_occlusion. When this property is set, we can use it to compute 12
patches to paint the shadow rather than 8 (center omitted). This prevents
drawing on the window's content completely and allows layer promotion
algorithms to promote the windows's content as layer if possible (in
particular with videos).
BUG=603866
TEST=cc_unittests --gtest_filter=NinePatchLayer*
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
Committed: https://crrev.com/8b134c7492731333fabeda5e358f0604ae5f565e
Cr-Commit-Position: refs/heads/master@{#389770}
Patch Set 1 #Patch Set 2 : Fix int/size_t compilation issue #
Total comments: 8
Patch Set 3 : Daniel's nits #
Total comments: 14
Patch Set 4 : sky & aelias' changes #
Total comments: 12
Patch Set 5 : Don't use pointers to std::vector #Patch Set 6 : Fix std::move errors #Patch Set 7 : Missing include #Patch Set 8 : switch to std::unique_ptr #Patch Set 9 : remove std::move #
Total comments: 1
Patch Set 10 : final nits #
Messages
Total messages: 40 (14 generated)
Description was changed from ========== cc: nine patch: add occlusion support When using a NinePatchLayer to draw shadows on windows on ChromeOS part of patches which are completely transparent are drawn on top of the window. This prevents for example videos to be promoted into layers. This patch introduce an optional property on the NinePatchLayer : layer_occlusion. When this property is set, we can use it to compute 12 patches to paint the shadow rather than 8 (center omitted). This prevents drawn on the window's content completely and allows layer promotion algorithms to promote the windows's content as layer if possible (in particular with videos). BUG=603866 TEST=cc_unittests --gtest_filter=NinePatchLayer* ========== to ========== cc: nine patch: add occlusion support When using a NinePatchLayer to draw shadows on windows on ChromeOS part of patches which are completely transparent are drawn on top of the window. This prevents for example videos to be promoted into layers. This patch introduce an optional property on the NinePatchLayer : layer_occlusion. When this property is set, we can use it to compute 12 patches to paint the shadow rather than 8 (center omitted). This prevents drawn on the window's content completely and allows layer promotion algorithms to promote the windows's content as layer if possible (in particular with videos). BUG=603866 TEST=cc_unittests --gtest_filter=NinePatchLayer* CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
lionel.g.landwerlin@intel.com changed reviewers: + avi@chromium.org, danakj@chromium.org, msw@chromium.org
danakj@chromium.org: Please review changes in cc/layers and ui/compositor. msw@chromium.org: Please review changes in mash. avi@chromium.org: Please review changes in ui/wm. Thanks!
Description was changed from ========== cc: nine patch: add occlusion support When using a NinePatchLayer to draw shadows on windows on ChromeOS part of patches which are completely transparent are drawn on top of the window. This prevents for example videos to be promoted into layers. This patch introduce an optional property on the NinePatchLayer : layer_occlusion. When this property is set, we can use it to compute 12 patches to paint the shadow rather than 8 (center omitted). This prevents drawn on the window's content completely and allows layer promotion algorithms to promote the windows's content as layer if possible (in particular with videos). BUG=603866 TEST=cc_unittests --gtest_filter=NinePatchLayer* CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: nine patch: add occlusion support When using a NinePatchLayer to draw shadows on windows on ChromeOS part of patches which are completely transparent are drawn on top of the window. This prevents for example videos to be promoted into layers. This patch introduce an optional property on the NinePatchLayer : layer_occlusion. When this property is set, we can use it to compute 12 patches to paint the shadow rather than 8 (center omitted). This prevents drawing on the window's content completely and allows layer promotion algorithms to promote the windows's content as layer if possible (in particular with videos). BUG=603866 TEST=cc_unittests --gtest_filter=NinePatchLayer* CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
avi@chromium.org changed reviewers: + derat@chromium.org, hshi@chromium.org - avi@chromium.org
I don't know this area of ui/. Based on blame, I'm tossing this to hshi and derat.
On 2016/04/15 14:21:23, Avi (OOO 20 April - 2 May) wrote: > I don't know this area of ui/. Based on blame, I'm tossing this to hshi and > derat. Oh, and when you get their OK I'll stamp.
derat@chromium.org changed reviewers: + aelias@chromium.org
i wrote similar code for chrome os a long time ago, but i'm afraid that this code is way beyond my understanding now. :-( here are some style comments, but you'll probably need to find someone else to do a true review. maybe aelias@ would be a good choice, based on https://chromiumcodereview.appspot.com/11304020? https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer.h (right): https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer.h:44: // |rect| is the space completly occluded by another layer in layer space. nit: s/completly/completely/ https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer.h:62: gfx::Rect layer_occlusion_; nit: add a brief comment describing this member (maybe just "Occluded area in layer space set by SetLayerOcclusion.") https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.cc (right): https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:49: (float)rect.x() / total_width, (float)rect.y() / total_height, please don't use c-style casts. use static_cast instead. but i don't understand why you need these casts. you're already dividing all of these int values by floats, which should automatically cast them. https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.h (right): https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.h:57: // |occlusion_rectangle| this comment doesn't do much good on its own. :-P please describe it in terms of the above diagrams, like the other parameters are described.
https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer.h (right): https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer.h:44: // |rect| is the space completly occluded by another layer in layer space. On 2016/04/15 15:38:45, Daniel Erat wrote: > nit: s/completly/completely/ Done. https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer.h:62: gfx::Rect layer_occlusion_; On 2016/04/15 15:38:45, Daniel Erat wrote: > nit: add a brief comment describing this member (maybe just "Occluded area in > layer space set by SetLayerOcclusion.") Done. https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.cc (right): https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:49: (float)rect.x() / total_width, (float)rect.y() / total_height, On 2016/04/15 15:38:45, Daniel Erat wrote: > please don't use c-style casts. use static_cast instead. > > but i don't understand why you need these casts. you're already dividing all of > these int values by floats, which should automatically cast them. Done. https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.h (right): https://codereview.chromium.org/1889153002/diff/20001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.h:57: // |occlusion_rectangle| On 2016/04/15 15:38:45, Daniel Erat wrote: > this comment doesn't do much good on its own. :-P please describe it in terms of > the above diagrams, like the other parameters are described. Done.
msw@chromium.org changed reviewers: + sky@chromium.org
Scott would be a better reviewer for the mash change.
LGTM with some docs https://codereview.chromium.org/1889153002/diff/40001/ui/compositor/layer.h File ui/compositor/layer.h (right): https://codereview.chromium.org/1889153002/diff/40001/ui/compositor/layer.h#n... ui/compositor/layer.h:307: void UpdateNinePatchOcclusion(const gfx::Rect& rect); These may make sense to you, but I've no idea what occlusion means in this context and what rect is.
Why is this new algorithm needed? Can't you get the same visual effect by changing the ninepatch resource and its geometry instead? That would be simpler and more efficient.
On 2016/04/18 22:33:23, aelias wrote: > Why is this new algorithm needed? Can't you get the same visual effect by > changing the ninepatch resource and its geometry instead? That would be simpler > and more efficient. I don't think this is possible because the size of the patches needs to match the image's content and respect the rounding in the corners. If we place the aperture at the wrong position, we'll end up stretching part of the shadow's resource that shouldn't be stretched and it will make this look wrong (especially on wide or long windows). I have considered reordering the layers (putting the shadow bellow the window's content as opposed to on top as it is currently), but there are lots of assumption on the what layer contains the window's content (in ash/content/ui directories).
Understood, I think I've convinced myself that your approach is needed. Because the ninepatch elements overlap with each other, it's not possible to use a single rectangle for the corner shadow that looks the same while avoiding the danger zone. No major objections then. https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer.h (right): https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer.h:47: void SetLayerOcclusion(const gfx::Rect& rect); nit: call the variable "occlusion", not "rect". https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer.h:62: // The occluded region in layer space set by SetLayerOcclusion. Would it be accurate to write "May be larger than the image aperture"? If so, please add that comment. https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.cc (right): https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:45: static gfx::RectF NormalizedRect(const gfx::Rect& rect, This argument change loses precision, use a gfx::RectF instead. (Probably most of this file should really be gfx::RectF, but let's not make it worse.) https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:93: void NinePatchLayerImpl::AppendQuadsWithoutOcclusion( These functions aren't actually appending the quads. Please rename them "ComputeQuadsWith...". https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:94: RenderPass* render_pass, This doesn't use render_pass, resource or shared_quad_state for anything, please delete those arguments. https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.h (right): https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.h:101: void AppendQuadsWithOcclusion(RenderPass* render_pass, These methods should be marked "const".
avi@chromium.org changed reviewers: - avi@chromium.org
I'm off to vacation, but it looks like the other reviewers have this under control. Don't wait for me :)
https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer.h (right): https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer.h:47: void SetLayerOcclusion(const gfx::Rect& rect); On 2016/04/20 05:54:57, aelias wrote: > nit: call the variable "occlusion", not "rect". Done. https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer.h:62: // The occluded region in layer space set by SetLayerOcclusion. On 2016/04/20 05:54:57, aelias wrote: > Would it be accurate to write "May be larger than the image aperture"? If so, > please add that comment. Done. https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.cc (right): https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:45: static gfx::RectF NormalizedRect(const gfx::Rect& rect, On 2016/04/20 05:54:57, aelias wrote: > This argument change loses precision, use a gfx::RectF instead. (Probably most > of this file should really be gfx::RectF, but let's not make it worse.) I could be missing something, but I don't think we're loosing precision here. All the arguments given to the previous NormalizedRect() function were integers (computed using gfx::Size image_bounds_ and gfx::Rect image_aperture_). https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:93: void NinePatchLayerImpl::AppendQuadsWithoutOcclusion( On 2016/04/20 05:54:57, aelias wrote: > These functions aren't actually appending the quads. Please rename them > "ComputeQuadsWith...". Done. https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:94: RenderPass* render_pass, On 2016/04/20 05:54:57, aelias wrote: > This doesn't use render_pass, resource or shared_quad_state for anything, please > delete those arguments. Done. https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.h (right): https://codereview.chromium.org/1889153002/diff/40001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.h:101: void AppendQuadsWithOcclusion(RenderPass* render_pass, On 2016/04/20 05:54:57, aelias wrote: > These methods should be marked "const". Done. https://codereview.chromium.org/1889153002/diff/40001/ui/compositor/layer.h File ui/compositor/layer.h (right): https://codereview.chromium.org/1889153002/diff/40001/ui/compositor/layer.h#n... ui/compositor/layer.h:307: void UpdateNinePatchOcclusion(const gfx::Rect& rect); On 2016/04/18 21:10:04, sky wrote: > These may make sense to you, but I've no idea what occlusion means in this > context and what rect is. Done.
https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.cc (right): https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:173: // Helper values. nit: this says "Helper values" and above says "Utility values". Please just delete both comments. https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:315: for (size_t i = 0; i < image_rects.size(); i++) { And this can be: for (NinePatchLayerImpl::Patch patch : patches) { https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.h (right): https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.h:83: const gfx::Rect& layer_occlusion); Please move this next to image_aperture in the argument order. https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.h:102: std::vector<gfx::Rect>* layer_rects) const; I think this pointer stuff should be avoided as follows: class Patch { Patch(gfx::Rect image_rect, gfx::Rect layer_rect); gfx::Rect image_rect; gfx::Rect layer_rect; }; std::vector<Patch> ComputeQuadsWithOcclusion(); (Note that because we are using C++11 STL, this should not incur an additional copy over your approach.)
https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.cc (right): https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:104: std::vector<NinePatchLayerImpl::Patches> patches; patches.reserve(kMaxPatches); https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:168: } return std::move(patches);
https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.cc (right): https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:104: On 2016/04/22 03:03:54, aelias wrote: > std::vector<NinePatchLayerImpl::Patches> patches; > patches.reserve(kMaxPatches); Done. https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:168: } On 2016/04/22 03:03:54, aelias wrote: > return std::move(patches); Done. https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:173: // Helper values. On 2016/04/22 03:00:42, aelias wrote: > nit: this says "Helper values" and above says "Utility values". Please just > delete both comments. Done. https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.cc:315: for (size_t i = 0; i < image_rects.size(); i++) { On 2016/04/22 03:00:42, aelias wrote: > And this can be: > > for (NinePatchLayerImpl::Patch patch : patches) { Done. https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... File cc/layers/nine_patch_layer_impl.h (right): https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.h:83: const gfx::Rect& layer_occlusion); On 2016/04/22 03:00:42, aelias wrote: > Please move this next to image_aperture in the argument order. Done. https://codereview.chromium.org/1889153002/diff/60001/cc/layers/nine_patch_la... cc/layers/nine_patch_layer_impl.h:102: std::vector<gfx::Rect>* layer_rects) const; On 2016/04/22 03:00:42, aelias wrote: > I think this pointer stuff should be avoided as follows: > > class Patch { > Patch(gfx::Rect image_rect, gfx::Rect layer_rect); > gfx::Rect image_rect; > gfx::Rect layer_rect; > }; > > std::vector<Patch> ComputeQuadsWithOcclusion(); > > (Note that because we are using C++11 STL, this should not incur an additional > copy over your approach.) Done.
This doesn't seem to be accepted by the compiler : return std::move(patches); Should we use a scopted_ptr<std::vector<Patch>> instead ?
On Fri, Apr 22, 2016 at 7:41 AM, <lionel.g.landwerlin@intel.com> wrote: > This doesn't seem to be accepted by the compiler : > > return std::move(patches); > > Should we use a scopted_ptr<std::vector<Patch>> instead ? > return patches; The std::move is not needed unless you are casting the type. Return makes patches into an rvalue. https://www.chromium.org/rvalue-references#TOC-11.-Don-t-return-references.-D... . -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Sounds good, I'm still trying to master the rvalue reference rules. This should be equally efficient if the unique_ptr<> is simply removed then and nothing special is done on the return, right, since vector<> has a move constructor in recent versions of the STL?
On Fri, Apr 22, 2016 at 5:53 PM, <aelias@chromium.org> wrote: > Sounds good, I'm still trying to master the rvalue reference rules. This > should > be equally efficient if the unique_ptr<> is simply removed then and nothing > special is done on the return, right, since vector<> has a move > constructor in > recent versions of the STL? > Right. making a std::vector and returning it will return an rvalue reference. The receiver's move constructor will just steal the pointer which is cheap (or RVO will just make the vector inside your function directly use the storage of the vector outside the function maybe even which is better yet). > > https://codereview.chromium.org/1889153002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
lgtm modulo remaining comment. https://codereview.chromium.org/1889153002/diff/160001/cc/layers/nine_patch_l... File cc/layers/nine_patch_layer_impl.cc (right): https://codereview.chromium.org/1889153002/diff/160001/cc/layers/nine_patch_l... cc/layers/nine_patch_layer_impl.cc:98: std::unique_ptr<std::vector<NinePatchLayerImpl::Patch>> OK then, please remove the std::unique pointers from this patch, since we've confirmed they don't do anything useful.
The CQ bit was checked by lionel.g.landwerlin@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org, aelias@chromium.org Link to the patchset: https://codereview.chromium.org/1889153002/#ps180001 (title: "final nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1889153002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1889153002/180001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to commit the patch.
The CQ bit was unchecked by commit-bot@chromium.org
The CQ bit was checked by lionel.g.landwerlin@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1889153002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1889153002/180001
Message was sent while issue was closed.
Description was changed from ========== cc: nine patch: add occlusion support When using a NinePatchLayer to draw shadows on windows on ChromeOS part of patches which are completely transparent are drawn on top of the window. This prevents for example videos to be promoted into layers. This patch introduce an optional property on the NinePatchLayer : layer_occlusion. When this property is set, we can use it to compute 12 patches to paint the shadow rather than 8 (center omitted). This prevents drawing on the window's content completely and allows layer promotion algorithms to promote the windows's content as layer if possible (in particular with videos). BUG=603866 TEST=cc_unittests --gtest_filter=NinePatchLayer* CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: nine patch: add occlusion support When using a NinePatchLayer to draw shadows on windows on ChromeOS part of patches which are completely transparent are drawn on top of the window. This prevents for example videos to be promoted into layers. This patch introduce an optional property on the NinePatchLayer : layer_occlusion. When this property is set, we can use it to compute 12 patches to paint the shadow rather than 8 (center omitted). This prevents drawing on the window's content completely and allows layer promotion algorithms to promote the windows's content as layer if possible (in particular with videos). BUG=603866 TEST=cc_unittests --gtest_filter=NinePatchLayer* CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
Message was sent while issue was closed.
Committed patchset #10 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== cc: nine patch: add occlusion support When using a NinePatchLayer to draw shadows on windows on ChromeOS part of patches which are completely transparent are drawn on top of the window. This prevents for example videos to be promoted into layers. This patch introduce an optional property on the NinePatchLayer : layer_occlusion. When this property is set, we can use it to compute 12 patches to paint the shadow rather than 8 (center omitted). This prevents drawing on the window's content completely and allows layer promotion algorithms to promote the windows's content as layer if possible (in particular with videos). BUG=603866 TEST=cc_unittests --gtest_filter=NinePatchLayer* CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: nine patch: add occlusion support When using a NinePatchLayer to draw shadows on windows on ChromeOS part of patches which are completely transparent are drawn on top of the window. This prevents for example videos to be promoted into layers. This patch introduce an optional property on the NinePatchLayer : layer_occlusion. When this property is set, we can use it to compute 12 patches to paint the shadow rather than 8 (center omitted). This prevents drawing on the window's content completely and allows layer promotion algorithms to promote the windows's content as layer if possible (in particular with videos). BUG=603866 TEST=cc_unittests --gtest_filter=NinePatchLayer* CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/8b134c7492731333fabeda5e358f0604ae5f565e Cr-Commit-Position: refs/heads/master@{#389770} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/8b134c7492731333fabeda5e358f0604ae5f565e Cr-Commit-Position: refs/heads/master@{#389770}
Message was sent while issue was closed.
A revert of this CL (patchset #10 id:180001) has been created in https://codereview.chromium.org/1983803003/ by lionel.g.landwerlin@intel.com. The reason for reverting is: Shadow are broken on HiDPI monitors. See https://bugs.chromium.org/p/chromium/issues/detail?id=607033 More people are noticing this and the fix is not ready yet. Let's revert this for now.. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
