New issue
Advanced search Search tips

Issue 912357 link

Starred by 1 user
Status: Fixed
Owner:
wangxianzhu@chromium.org
Closed: Jan 3
Cc:
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Video overflow is not clipped

Project Member Reported by wangxianzhu@google.com, Dec 5 
Chrome Version: ToT

This reflects the following TODO at TODO(trchen) in https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/video_painter.cc?rcl=bbbb99807779dc6296a7993a3fa0fc34e04024bc&l=64:

// TODO(trchen): Video rect could overflow the content rect due to object-fit.
// Should apply a clip here like EmbeddedObjectPainter does.

However, the comment has been out-dated since we added OverflowClip property for replaced elements. Now the problem is that LayoutVideo is a LayoutImage which is supposed to support embedded clip in its painter but VideoPainter doens't.

The correct way is to apply the clip in ScopedReplacedContentPaintState (in replaced_painter.cc), and remove other unnecessary embedded clips (like the one in EmbeddedObjectPainter).
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50bef70407e97669c659621aafe2f9f14e162489

commit 50bef70407e97669c659621aafe2f9f14e162489
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Thu Jan 03 22:59:44 2019

[CompositeAfterPaint] Fix video overflow clip

Previously video with object-fit:none or object-fit:cover was
not clipped when the contents overflowed the replaced content
box, because it was treated as an image that was supposed
not to paint outside of the replaced content box but not true.

Simplify overflow clip logic of replaced element. Remove the
dependency of image overflow clip on compositing state.

Now media no longer inherit overflow clip rule from image to
ensure overflowing contents of media are clipped.

ReplacedPainter will emit paint chunk with overflow clip
depending on paint property only. No longer depends on other
conditions. This applies to all replaced elements.

Bug:  912357 
Change-Id: I5de102f7b6b9ad79f372392de44012deb8a9cfec
Reviewed-on: https://chromium-review.googlesource.com/c/1387152
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619797}
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/renderer/core/paint/embedded_object_painter.cc
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/renderer/core/paint/paint_property_tree_update_tests.cc
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/renderer/core/paint/replaced_painter.cc
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/renderer/core/paint/video_painter.cc
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/web_tests/FlagExpectations/enable-blink-features=CompositeAfterPaint
[add] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/web_tests/compositing/lots-of-img-layers-expected.png
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/web_tests/flag-specific/enable-blink-features=CompositeAfterPaint/media/video-zoom-controls-expected.png
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/web_tests/platform/linux/compositing/direct-image-compositing-expected.png
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/web_tests/platform/mac/compositing/direct-image-compositing-expected.png
[delete] https://crrev.com/7b1dc231b146ada6a1374fc710ca8675c34ee7ac/third_party/blink/web_tests/platform/mac/compositing/lots-of-img-layers-expected.png
[modify] https://crrev.com/50bef70407e97669c659621aafe2f9f14e162489/third_party/blink/web_tests/platform/win/compositing/direct-image-compositing-expected.png
[delete] https://crrev.com/7b1dc231b146ada6a1374fc710ca8675c34ee7ac/third_party/blink/web_tests/platform/win/compositing/lots-of-img-layers-expected.png

Comment 2 by wangxianzhu@chromium.org, Jan 3

Status: Fixed (was: Assigned)
►

Sign in to add a comment