Bug 250882

Summary:	BackgroundPainter::paintFillLayers should check box-shadow when finding an opaque layer.
Product:	WebKit	Reporter:	Ahmad Saleem <ahmad.saleem792>
Component:	Layout and Rendering	Assignee:	Nobody <webkit-unassigned>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	bfulgham, karlcow, koivisto, ntim, simon.fraser, webkit-bug-importer, zalan
Priority:	P2	Keywords:	BrowserCompat, InRadar
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified

Description Ahmad Saleem 2023-01-19 17:49:38 PST

Hi Team,

While going through Blink's commit, I came across another failing test case in Safari 16.2 and STP161:

Test Case - https://jsfiddle.net/hL9bj0q7/show

^ It does not have any shadow in Safari but does in Chrome Canary 111 and Firefox Nightly 111.

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/a48f4540763c24adff42b9b77d719597f51482f6

WebKit Source - https://searchfox.org/wubkat/source/Source/WebCore/rendering/BackgroundPainter.cpp#131

Need to add below or equivalent:

!boxShadowShouldBeAppliedToBackground(bleedAvoidance) 

OR

!boxShadowShouldBeAppliedToBackground(m_renderer, paintRect.location(), bleedAvoidance, { })

____

Just wanted to raise so we can fix it.

Thanks!

Comment 1 Karl Dubost 2023-01-19 20:43:19 PST

There is a FIXME in the source code. 
https://searchfox.org/wubkat/rev/5b1acc527ab5b10c9d9e4dd21fcec68e315790c2/Source/WebCore/rendering/BackgroundPainter.cpp#122-132

And the code was last edited by antti
but the FIXME predates this 

Adding occlusion detection to reduce overdraw in RenderBox background rendering 
Justin Novosad, Fri, 23 Nov 2012 18:43:51 +0000
added in https://searchfox.org/wubkat/commit/71cbe979b543bafb933abdd82a03d72df95c7e0e

// Stop traversal when an opaque layer is encountered.
// FIXME: It would be possible for the following occlusion culling test to be more aggressive
// on layers with no repeat by testing whether the image covers the layout rect.
// Testing that here would imply duplicating a lot of calculations that are currently done in
// BackgroundPainter::paintFillLayer. A more efficient solution might be to move
// the layer recursion into paintFillLayer, or to compute the layer geometry here
// and pass it down.

// The clipOccludesNextLayers condition must be evaluated first to avoid short-circuiting.
if (layer->clipOccludesNextLayers(layer == &fillLayer) && layer->hasOpaqueImage(m_renderer) && layer->image()->canRender(&m_renderer, m_renderer.style().effectiveZoom()) && layer->hasRepeatXY() && layer->blendMode() == BlendMode::Normal)
	break;

Comment 2 Radar WebKit Bug Importer 2023-01-26 17:50:17 PST

<rdar://problem/104722307>

Comment 3 Ahmad Saleem 2024-03-04 11:12:54 PST

This compiles and fixes the bug:

&& !boxShadowShouldBeAppliedToBackground(m_renderer, rect.location(), bleedAvoidance, { })

___

Just wanted to share.

(Draft) PR - https://github.com/WebKit/WebKit/pull/25436

Comment 4 EWS 2024-03-06 02:10:05 PST

Committed 275735@main (b6231f750f4e): <https://commits.webkit.org/275735@main>

Reviewed commits have been landed. Closing PR #25436 and removing active labels.