Closed Bug 1544798 Opened 4 months ago Closed 4 months ago

FrameLayerBuilder.cpp DebugPaintItem() crashes if item bounds has dimension less than 1.0 pixels

Categories

(Core :: Web Painting, defect, P3)

x86_64
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: cmartin, Assigned: miko)

Details

Attachments

(3 files)

Attached file patch.txt

On this line of DebugPaintItem() it truncates the result of aItem->GetBounds() to the nearest int32_t.

Unfortunately, if the result of that truncation is 0, tempDT will be nullptr, leading to gfxContext failing, leading to the InvalidContext crash.

I have been able to work around it for now using the attached patch, but I don't know if it's better to make the code handle the sub-pixel case properly - Possibly by changing from truncation to round-up or forcing a 1-pixel minimum?

Below is the stack trace from the failed item. It happens after the browser first shows, but before it becomes interactive.

Crash Annotation GraphicsCriticalError: |[0][GFX1-]: Invalid target in gfxContext::CreateOrNull 0000000000000000 (t=73.7136) [GFX1-]: Invalid target in gfxContext::CreateOrNull 0000000000000000
Crash Annotation GraphicsCriticalError: |[0][GFX1-]: Invalid target in gfxContext::CreateOrNull 0000000000000000 (t=73.7136) |[1][GFX1 28]: DebugPaintItem context problem 0000000000000000 (t=73.7136) [GFX1 28]: DebugPaintItem context problem 0000000000000000
Assertion failure: [GFX1 28]: DebugPaintItem context problem 0000000000000000, at c:/moz/mozilla-central/obj-x86_64-pc-mingw32/dist/include/mozilla/gfx/Logging.h:747
++DOCSHELL 000001C90C545800 == 1 [pid = 16264] [id = {5884ac38-45ec-4861-b76d-b45685460b22}]
++DOMWINDOW == 1 (000001C90EB9F020) [pid = 16264] [serial = 1] [outer = 0000000000000000]
#01: mozilla::gfx::Log<1,mozilla::gfx::CriticalLogger>::Flush (c:\moz\mozilla-central\obj-x86_64-pc-mingw32\dist\include\mozilla\gfx\Logging.h:288)
#02: mozilla::gfx::Log<1,mozilla::gfx::CriticalLogger>::~Log (c:\moz\mozilla-central\obj-x86_64-pc-mingw32\dist\include\mozilla\gfx\Logging.h:279)
#03: mozilla::DebugPaintItem (c:\moz\mozilla-central\layout\painting\FrameLayerBuilder.cpp:6476)
#04: mozilla::FrameLayerBuilder::PaintItems (c:\moz\mozilla-central\layout\painting\FrameLayerBuilder.cpp:7012)
#05: mozilla::FrameLayerBuilder::DrawPaintedLayer (c:\moz\mozilla-central\layout\painting\FrameLayerBuilder.cpp:7185)
#06: mozilla::layers::BasicPaintedLayer::PaintThebes (c:\moz\mozilla-central\gfx\layers\basic\BasicPaintedLayer.cpp:92)
#07: mozilla::layers::BasicLayerManager::PaintSelfOrChildren (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:690)
#08: mozilla::layers::BasicLayerManager::PaintLayer (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:869)
#09: mozilla::layers::BasicLayerManager::PaintSelfOrChildren (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:711)
#10: mozilla::layers::BasicLayerManager::PaintLayer (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:869)
#11: mozilla::layers::BasicLayerManager::EndTransactionInternal (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:605)
#12: mozilla::layers::BasicLayerManager::EndTransaction (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:538)
#13: mozilla::PaintInactiveLayer (c:\moz\mozilla-central\layout\painting\FrameLayerBuilder.cpp:4117)
#14: mozilla::FrameLayerBuilder::PaintItems (c:\moz\mozilla-central\layout\painting\FrameLayerBuilder.cpp:7003)
#15: mozilla::FrameLayerBuilder::DrawPaintedLayer (c:\moz\mozilla-central\layout\painting\FrameLayerBuilder.cpp:7185)
#16: mozilla::layers::BasicPaintedLayer::PaintThebes (c:\moz\mozilla-central\gfx\layers\basic\BasicPaintedLayer.cpp:92)
#17: mozilla::layers::BasicLayerManager::PaintSelfOrChildren (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:690)
#18: mozilla::layers::BasicLayerManager::PaintLayer (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:869)
#19: mozilla::layers::BasicLayerManager::PaintSelfOrChildren (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:711)
#20: mozilla::layers::BasicLayerManager::PaintLayer (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:869)
#21: mozilla::layers::BasicLayerManager::EndTransactionInternal (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:605)
#22: mozilla::layers::BasicLayerManager::EndTransaction (c:\moz\mozilla-central\gfx\layers\basic\BasicLayerManager.cpp:538)
#23: nsDisplayList::PaintRoot (c:\moz\mozilla-central\layout\painting\nsDisplayList.cpp:2864)
#24: nsLayoutUtils::PaintFrame (c:\moz\mozilla-central\layout\base\nsLayoutUtils.cpp:3986)
#25: mozilla::PresShell::RenderDocument (c:\moz\mozilla-central\layout\base\PresShell.cpp:4514)
#26: mozilla::image::SVGDrawingCallback::operator() (c:\moz\mozilla-central\image\VectorImage.cpp:308)
#27: gfxCallbackDrawable::Draw (c:\moz\mozilla-central\gfx\thebes\gfxDrawable.cpp:149)
#28: gfxUtils::DrawPixelSnapped (c:\moz\mozilla-central\gfx\thebes\gfxUtils.cpp:564)
#29: mozilla::image::imgFrame::InitWithDrawable (c:\moz\mozilla-central\image\imgFrame.cpp:435)
#30: mozilla::image::VectorImage::CreateSurface (c:\moz\mozilla-central\image\VectorImage.cpp:1127)
#31: mozilla::image::VectorImage::GetFrameInternal (c:\moz\mozilla-central\image\VectorImage.cpp:794)
#32: mozilla::image::ImageResource::GetImageContainerImpl (c:\moz\mozilla-central\image\Image.cpp:189)
#33: mozilla::image::VectorImage::GetImageContainerAtSize (c:\moz\mozilla-central\image\VectorImage.cpp:877)
#34: nsImageBoxFrame::CreateWebRenderCommands (c:\moz\mozilla-central\layout\xul\nsImageBoxFrame.cpp:425)
#35: nsDisplayXULImage::CreateWebRenderCommands (c:\moz\mozilla-central\layout\xul\nsImageBoxFrame.cpp:535)
#36: mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList (c:\moz\mozilla-central\gfx\layers\wr\WebRenderCommandBuilder.cpp:1730)
#37: mozilla::layers::WebRenderCommandBuilder::BuildWebRenderCommands (c:\moz\mozilla-central\gfx\layers\wr\WebRenderCommandBuilder.cpp:1564)
#38: mozilla::layers::WebRenderLayerManager::EndTransactionWithoutLayer (c:\moz\mozilla-central\gfx\layers\wr\WebRenderLayerManager.cpp:327)
#39: nsDisplayList::PaintRoot (c:\moz\mozilla-central\layout\painting\nsDisplayList.cpp:2763)
#40: nsLayoutUtils::PaintFrame (c:\moz\mozilla-central\layout\base\nsLayoutUtils.cpp:3986)
#41: mozilla::PresShell::Paint (c:\moz\mozilla-central\layout\base\PresShell.cpp:6063)
#42: nsViewManager::ProcessPendingUpdatesPaint (c:\moz\mozilla-central\view\nsViewManager.cpp:462)
#43: nsViewManager::ProcessPendingUpdatesForView (c:\moz\mozilla-central\view\nsViewManager.cpp:399)
#44: nsViewManager::ProcessPendingUpdates (c:\moz\mozilla-central\view\nsViewManager.cpp:1021)
#45: nsRefreshDriver::Tick (c:\moz\mozilla-central\layout\base\nsRefreshDriver.cpp:2060)
#46: mozilla::RefreshDriverTimer::TickDriver (c:\moz\mozilla-central\layout\base\nsRefreshDriver.cpp:348)
#47: mozilla::RefreshDriverTimer::TickRefreshDrivers (c:\moz\mozilla-central\layout\base\nsRefreshDriver.cpp:319)
#48: mozilla::RefreshDriverTimer::Tick (c:\moz\mozilla-central\layout\base\nsRefreshDriver.cpp:344)
#49: mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers (c:\moz\mozilla-central\layout\base\nsRefreshDriver.cpp:788)
#50: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver (c:\moz\mozilla-central\layout\base\nsRefreshDriver.cpp:710)
#51: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run (c:\moz\mozilla-central\layout\base\nsRefreshDriver.cpp:508)
#52: nsThread::ProcessNextEvent (c:\moz\mozilla-central\xpcom\threads\nsThread.cpp:1182)
Flags: needinfo?(jbonisteel)

This code is meant for debugging purposes only.
Does adding

  if (bounds.IsEmpty()) {
    return;
  }

after

  Rect bounds = NSRectToRect(aItem->GetBounds(aBuilder, &snap),
                             aPresContext->AppUnitsPerDevPixel());

fix this for your use case?

Component: Graphics: Layers → Web Painting
Flags: needinfo?(jbonisteel)
Priority: -- → P3

(In reply to Miko Mynttinen [:miko] from comment #1)

This code is meant for debugging purposes only.
Does adding

  if (bounds.IsEmpty()) {
    return;
  }

after

  Rect bounds = NSRectToRect(aItem->GetBounds(aBuilder, &snap),
                             aPresContext->AppUnitsPerDevPixel());

fix this for your use case?

It didn't work unfortunately, because the bounds are not 'empty' at that point. They are both floats, and the height in my case is something like "0.5f", which gets truncated to "0" when it is cast to an int32_t in the line below.

But is truncation the correct behavior in this case? If you have an object that is "10.5W x 7.5H", don't you need a surface that is "11W x 8H" to hold it?

Flags: needinfo?(mikokm)

(In reply to Chris Martin [:cmartin] from comment #2)

(In reply to Miko Mynttinen [:miko] from comment #1)

This code is meant for debugging purposes only.
Does adding

  if (bounds.IsEmpty()) {
    return;
  }

after

  Rect bounds = NSRectToRect(aItem->GetBounds(aBuilder, &snap),
                             aPresContext->AppUnitsPerDevPixel());

fix this for your use case?

It didn't work unfortunately, because the bounds are not 'empty' at that point. They are both floats, and the height in my case is something like "0.5f", which gets truncated to "0" when it is cast to an int32_t in the line below.

But is truncation the correct behavior in this case? If you have an object that is "10.5W x 7.5H", don't you need a surface that is "11W x 8H" to hold it?

Ah you are correct, the problem is truncating the size below this. I have attached a patch that should handle this.

Flags: needinfo?(mikokm)
Attached patch 1544798.patchSplinter Review
Assignee: nobody → mikokm
Status: NEW → ASSIGNED

Ah - Thank you, that patch seems to fix the issue :)

Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/44870572e85a
Do not paint items with empty truncated bounds in DebugPaintItem() r=kamidphish
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.