New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 725995 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

SPv2 makes layers with no PaintOps in them

Project Member Reported by danakj@chromium.org, May 24 2017

Issue description

In DisplayItemList::Raster we do rtree_.Search() to get the set of visual rects that match the query.

https://cs.chromium.org/chromium/src/cc/paint/display_item_list.cc?rcl=ab4f7f57c0f31fbdb203db09acbf3a3a332e22b0&l=196

I was trying to add a DCHECK that the result is not empty but was surprised to find it is for a bunch SPv2 layout tests, but not for SPv1, such as  compositing/fixed-background-composited-html.html

STDERR: [1:1:0524/093654.601626:5520618573:FATAL:paint_op_buffer.cc(677)] Check failed: !range_indices.empty(). 
STDERR: #0 0x00000199df67 base::debug::StackTrace::StackTrace()
STDERR: #1 0x0000019b537d logging::LogMessage::~LogMessage()
STDERR: #2 0x0000021a022e cc::PaintOpBuffer::PlaybackRanges()
STDERR: #3 0x00000219e253 cc::DisplayItemList::Raster()
STDERR: #4 0x0000028bae2d cc::RecordingSource::DetermineIfSolidColor()
STDERR: #5 0x0000028baaf9 cc::RecordingSource::FinishDisplayItemListUpdate()

Here is is querying the entire layer in order to check if solid color.

It appears that PaintChunksToCcLayer::Convert() adds the translate but then nothing else in this case. There is 1 paint chunk and 2 display_items, with 1 matching the paint chunk. However AppendDisplayitemToCcDisplayItemList() is called with the display item and aborts cuz the display item's GetPaintRecord() is null.

I do think this is probably a reasonable DCHECK to have but I'll avoid it for now.

https://chromium-review.googlesource.com/c/506430#message-607eead3c6d0b7bb9b7af0ea775bd83024f58d54 this is where it came up.
 

Comment 1 by rtoy@chromium.org, May 24 2017

Components: -Blink
Labels: BugSource-Chromium PaintTeamTriaged-20170525
The PaintOpBuffer looks like it's been reworked since above. What is the latest way to add a similar DCHECK?

I tried naively adding:

  DCHECK(!offsets.empty());

to cc::DisplayItemList::Raster and it passes for the cited compositing/fixed-background-composited-html.html test with both SPv1 and SPv2 but I'm not sure if it's an analogous check vs the one in OP. I know this area of code has been under heavy work by like four people recently so seeking checkpoint rather than digging further.

Comment 4 by enne@chromium.org, Aug 7 2017

If there's really no ops at all, then it's just PaintOpBuffer::size.

It's not clear to me that if something has no ops inside of a solid color analysis rect that something is amiss.  That seems like it could be possible for some arbitrary rect.
Cc: trchen@chromium.org
Status: WontFix (was: Assigned)
Closing as it's not clear this is a problem but please reopen if further investigation desired. Also +trchen as he may be interested or have thoughts.

Sign in to add a comment