Closed Bug 1393383 Opened 2 years ago Closed 2 years ago
Support non-uniform radius for box shadow
59 bytes, text/x-review-board-request
For , we should add the non-uniform radius support in WR and remove the limitation in gecko.  https://dxr.mozilla.org/mozilla-central/rev/f0abd25e1f4acced652d180c34b7c9eda638deb1/layout/painting/nsDisplayList.cpp#5411
Priority: -- → P3
Priority: P3 → P2
Whiteboard: [gfx-noted] → [wr-mvp] [gfx-noted]
WebRender support for this landed in https://github.com/servo/webrender/pull/1889. However, we'll need a webrender update after bug 1408461 to get it into Gecko.
Submit a try run for it. https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ebd7d5c9f7babdec42db0803b9076016374632e
Attachment #8920028 - Flags: review?(bugmail)
Hi Kats, WebRender added support for non-uniform border radius in this commit. https://github.com/servo/webrender/commit/ddc859f95314c07f486f94c2ec2f6ccdead8a1f7 It needs update in gecko side code, too. This patch removed uniform border radius constraint when creating corresponding webrender commands and update parameter types in webrender_binding code.
The patch looks reasonable but can you rebase it on the latest central code? It looks like this is based on code from before the webrender update landed. You will need to change another line in the bindings and also use LayoutDeviceSize instead of gfx::Size.
(You should also drop that first patch that updates webrender, since that is in m-c already)
Attachment #8920027 - Attachment is obsolete: true
OK, I have rebased it to latest central code and discard webrender update part. But I didn't see additional change required inside webrender_binding code.
Oh! For some reason I thought servo/webrender#1889 got merged with the last WR update, but it didn't. That's why I was confused, I knew that I had changed the line in  and didn't see you update that line in your patch. So you still have to wait until the next WR update into gecko before you can land this (and at that point you will need to change the line in  back from BorderRadius::uniform(border_radius) to just border_radius).  https://hg.mozilla.org/try/rev/093b74df4eff9e7699659b9504f4afb986148f7f
Comment on attachment 8920028 [details] Bug 1393383 - Update webrender binding code to support non-uniform border radius https://reviewboard.mozilla.org/r/191016/#review196348 I'll r+ the patch for now, see comments below. ::: layout/forms/nsButtonFrameRenderer.cpp:214 (Diff revision 2) > wr::LayoutRect deviceClipRect = aSc.ToRelativeLayoutRect(clipRect); > > bool hasBorderRadius; > Unused << nsCSSRendering::HasBoxShadowNativeTheme(mFrame, hasBorderRadius); > > - float borderRadius = 0.0; > + LayoutDeviceSize zeroSize(0.0,0.0); nit: add a space after the comma. Actually you can just leave this as "LayoutDeviceSize zeroSize;" because it will be initialized to 0 by default. ::: layout/forms/nsButtonFrameRenderer.cpp:222 (Diff revision 2) > - MOZ_ASSERT(borderRadii.AreRadiiSame()); > - borderRadius = hasBorderRadius ? borderRadii.TopLeft().width : 0.0; > + borderRadius = wr::ToBorderRadius( > + LayoutDeviceSize::FromUnknownSize(borderRadii.TopLeft()), Don't you need to check hasBorderRadius again here? ::: layout/painting/nsDisplayList.cpp:5405 (Diff revision 2) > LayoutDeviceRect deviceBox = LayoutDeviceRect::FromAppUnits( > shadowRect, appUnitsPerDevPixel); > wr::LayoutRect deviceBoxRect = aSc.ToRelativeLayoutRect(deviceBox); > wr::LayoutRect deviceClipRect = aSc.ToRelativeLayoutRect(clipRect); > > - // TODO: support non-uniform border radius. > + LayoutDeviceSize zeroSize(0.0,0.0); Ditto here, you don't need to explicitly initialize this
Attachment #8920028 - Flags: review?(bugmail) → review+
Whiteboard: [wr-mvp] [gfx-noted] → [wr-reserve] [gfx-noted]
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/f1f825dbd4cf Update webrender binding code to support non-uniform border radius r=kats
You need to log in before you can comment on or make changes to this bug.