[webkit-reviews] review requested: [Bug 202049] Setting border-radius on <video> element clips top and left sections of video : [Attachment 381389] Patch

Mon Oct 21 10:19:45 PDT 2019

Said Abou-Hallawa <sabouhallawa at apple.com> has asked  for review:
Bug 202049: Setting border-radius on <video> element clips top and left
sections of video
https://bugs.webkit.org/show_bug.cgi?id=202049

Attachment 381389: Patch

https://bugs.webkit.org/attachment.cgi?id=381389&action=review

--- Comment #6 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 381389
  --> https://bugs.webkit.org/attachment.cgi?id=381389
Patch

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

> Source/WebCore/platform/graphics/FloatRoundedRect.h:106
> +    void setLocation(FloatPoint location) { m_rect.setLocation(location); }

Can't we use setRect() or move() to change the location?

Or can't we make rect() non-const and use rect().setLocation()? Maybe a better
solution is to make FloatRoundedRect inherits FloatRect.

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2648
> +	   clippingRectRelativeToClippingLayer.setLocation({ });

Shouldn't clippingRectRelativeToClippingLayer be translated to the coordinates
of m_contentsClippingLayer instead of just making its location at { 0, 0 }?

> LayoutTests/compositing/video/video-border-radius-clipping-expected.html:27
> +	       var totalCount = document.getElementsByTagName('video').length;
> +	       var count = totalCount;

There is only one <video> element in the <body> element. Why do we have to get
the count and check if (!--count...) in the script below?

> LayoutTests/compositing/video/video-border-radius-clipping.html:28
> +	       var totalCount = document.getElementsByTagName('video').length;
> +	       var count = totalCount;

Ditto.