I don't think any user SVG code can generate a FilterNode graph that triggers this assert [1] right now. However, I'm expecting some FilterNode graphs, generated by a patch I'm trying to add in bug 948265, to work properly. The assert checks that GetData() was not called before Map() on a DataSourceSurfaceD2DTarget. The comment above the assert says: // DataSourceSurfaces used with the new Map API should not be used with GetData!! To reproduce the assert, you must apply patches 1 & 2, and load the reproduction on Win7. (Note patch 1 may be committed in the near future; patch 2 is needed to trigger the assert.) The reproduction uses a simple filter: <filter id="f1"> <feComposite in="SourceAlpha" in2="SourceGraphic"/> </filter> In bug 948265, I'm trying to refactor the way SourceAlpha inputs are handled, which changes the FilterNode filter graph that is constructed for the reproduction filter. Importantly, a distinct surface is not created for the SourceAlpha input anymore. Rather, the SourceGraphic surface is referenced again. With the new code, the following FilterNodeSoftware graph is created and executed: Premultiply DiscreteTransfer Unpremultiply Crop Composite Premultiply DiscreteTransfer Unpremultiply // Calls GetData() on the SourceGraphic surface [2]. Transform // Identity transform just returns the SourceGraphic surface to Unpremultiply above [3]. (SourceGraphic surface) Premultiply DiscreteTransfer Unpremultiply Crop DiscreteTransfer Transform // ASSERT! Calls Map() on the SourceGraphic surface [1], bug GetData() was called before. (SourceGraphic surface) // Without the patches, this would be (SourceAlpha surface). Without the patches, the second Transform FilterNode references a SourceAlpha surface different than the SourceGraphic surface. This avoids the assert in Map() because GetData() was not called previously on the SourceAlpha surface (only on the SourceGraphic surface). Note that FilterNodeSoftware has to be using DataSourceSurfaceD2DTarget for the assert to occur (since the assert is in DataSourceSurfaceD2DTarget). If FilterNodeSoftware uses SourceSurfaceCairo, it won't occur. I'm not sure what system conditions ensure FilterNodeSoftware uses DataSourceSurfaceD2DTarget. If you disable D2D, it'll use SourceSurfaceCairo. If you enable D2D, I typically get FilterNodeD2D1(s), not FilterNodeSoftware(s). Only if D2D is enabled and a call to get the D2D context in DrawTargetD2D::CreateFilter [4] fails will you get FilterNodeSoftware(s). Originally, my Win7 system was using FilterNodeSofware + DataSourceSurfaceD2DTarget for some reason, and I was able to reproduce the assert. Then, my system updated overnight and started using FilterNodeD2D1, so to reproduce the assert again, I had to comment out code in DrawTargetD2D::CreateFilter [4] and DrawTargetD2D::DrawFilter [5] to force FilterNodeSoftware + DataSourceSurfaceD2DTarget usage. The bots reliably reproduce the assert, though. I'm guessing the right fix is to use the new Map() API everywhere (particularly in Unpremultiply [2]) instead of GetData(), which sounds like it's being worked on, but I'm as not familiar with this part of the code. If that's the case, I'll defer my patch that's triggering the assert until the API usage changes. My patch is just refactoring, so it's not blocking anything. [1]: http://dxr.mozilla.org/mozilla-central/source/gfx/2d/SourceSurfaceD2DTarget.cpp#257 [2]: http://dxr.mozilla.org/mozilla-central/source/gfx/2d/FilterNodeSoftware.cpp#1343 [3]: http://dxr.mozilla.org/mozilla-central/source/gfx/2d/FilterNodeSoftware.cpp#1104 [4]: http://dxr.mozilla.org/mozilla-central/source/gfx/2d/DrawTargetD2D.cpp#1320 [5]: http://dxr.mozilla.org/mozilla-central/source/gfx/2d/DrawTargetD2D.cpp#379

Bas, what do think about this?

Thanks for tracking this down! (In reply to Max Vujovic from comment #0) > I'm guessing the right fix is to use the new Map() API everywhere > (particularly in Unpremultiply [2]) instead of GetData() Yes, I think that's what we should do.

(In reply to Markus Stange [:mstange] from comment #5) > Thanks for tracking this down! > > (In reply to Max Vujovic from comment #0) > > I'm guessing the right fix is to use the new Map() API everywhere > > (particularly in Unpremultiply [2]) instead of GetData() > > Yes, I think that's what we should do. Correct!