562574 - During test_bug539565-1.html: "WARNING **: expose rectangle not in clip rectangle" and "###!!! ASSERTION: Clip rectangle offsets are negative!: 'offsetX >= 0 && offsetY >= 0', file nsObjectFrame.cpp:5462"

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

STR: run the test in a debug build.

hg bisect says this started with http://hg.mozilla.org/mozilla-central/rev/c14e08cf3e61, which unfortunately was checked in one day after the cairo update.  I'll apply this to the rev before the cairo update.

Stack

** (<unknown>:3624): WARNING **: expose rectangle not in clip rectangle
###!!! ASSERTION: Clip rectangle offsets are negative!: 'offsetX >= 0 && offsetY >= 0', file /home/cjones/mozilla/mozilla-central/layout/generic/nsObjectFrame.cpp, line 5462
nsPluginInstanceOwner::Renderer::NativeDraw(_GdkDrawable*, short, short, _GdkRectangle*, unsigned int) (/home/cjones/mozilla/mozilla-central/layout/generic/nsObjectFrame.cpp:5463)
NativeRendering (/home/cjones/mozilla/mozilla-central/gfx/thebes/src/gfxGdkNativeRenderer.cpp:67)
_draw_with_xlib_direct (/home/cjones/mozilla/mozilla-central/gfx/thebes/src/cairo-xlib-utils.c:309)
cairo_draw_with_gdk (/home/cjones/mozilla/mozilla-central/gfx/thebes/src/cairo-xlib-utils.c:529)
gfxGdkNativeRenderer::Draw(gfxContext*, int, int, unsigned int, gfxGdkNativeRenderer::DrawOutput*) (/home/cjones/mozilla/mozilla-central/gfx/thebes/src/gfxGdkNativeRenderer.cpp:111)
nsPluginInstanceOwner::Paint(gfxContext*, gfxRect const&, gfxRect const&) (/home/cjones/mozilla/mozilla-central/layout/generic/nsObjectFrame.cpp:5101)
nsObjectFrame::PaintPlugin(nsIRenderingContext&, nsRect const&, nsRect const&) (/home/cjones/mozilla/mozilla-central/layout/generic/nsObjectFrame.cpp:1922)
nsDisplayPlugin::Paint(nsDisplayListBuilder*, nsIRenderingContext*) (/home/cjones/mozilla/mozilla-central/layout/generic/nsObjectFrame.cpp:1176)
nsDisplayList::PaintThebesLayers(nsDisplayListBuilder*, nsTArray<nsDisplayList::LayerItems> const&) const (/home/cjones/mozilla/mozilla-central/layout/base/nsDisplayList.cpp:837)
nsDisplayList::Paint(nsDisplayListBuilder*, nsIRenderingContext*, unsigned int) const (/home/cjones/mozilla/mozilla-central/layout/base/nsDisplayList.cpp:779)
nsLayoutUtils::PaintFrame(nsIRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) (/home/cjones/mozilla/mozilla-central/layout/base/nsLayoutUtils.cpp:1259)
PresShell::Paint(nsIView*, nsIView*, nsIWidget*, nsRegion const&, int) (/home/cjones/mozilla/mozilla-central/layout/base/nsPresShell.cpp:5630)
nsViewManager::RenderViews(nsView*, nsIWidget*, nsRegion const&) (/home/cjones/mozilla/mozilla-central/view/src/nsViewManager.cpp:455)
nsViewManager::Refresh(nsView*, nsIWidget*, nsIntRegion const&, unsigned int) (/home/cjones/mozilla/mozilla-central/view/src/nsViewManager.cpp:428)
nsViewManager::DispatchEvent(nsGUIEvent*, nsIView*, nsEventStatus*) (/home/cjones/mozilla/mozilla-central/view/src/nsViewManager.cpp:878)
HandleEvent (/home/cjones/mozilla/mozilla-central/view/src/nsView.cpp:160)
nsWindow::DispatchEvent(nsGUIEvent*, nsEventStatus&) (/home/cjones/mozilla/mozilla-central/widget/src/gtk2/nsWindow.cpp:587)
nsWindow::OnExposeEvent(_GtkWidget*, _GdkEventExpose*) (/home/cjones/mozilla/mozilla-central/widget/src/gtk2/nsWindow.cpp:2494)
expose_event_cb (/home/cjones/mozilla/mozilla-central/widget/src/gtk2/nsWindow.cpp:5704)
_gtk_marshal_BOOLEAN__BOXED (/build/buildd/gtk+2.0-2.18.3/gtk/gtkmarshalers.c:88)
closure_invoke_notifiers (/build/buildd/glib2.0-2.22.3/gobject/gclosure.c:285)
accumulate (/build/buildd/glib2.0-2.22.3/gobject/gsignal.c:3089)
IA__g_signal_emit_valist (/build/buildd/glib2.0-2.22.3/gobject/gsignal.c:2992)
IA__g_signal_emit (/build/buildd/glib2.0-2.22.3/gobject/gsignal.c:3039)
gtk_widget_event_internal (/build/buildd/gtk+2.0-2.18.3/gtk/gtkwidget.c:4767)
IA__gtk_main_do_event (/build/buildd/gtk+2.0-2.18.3/gtk/gtkmain.c:1582)
_gdk_window_process_updates_recurse (/build/buildd/gtk+2.0-2.18.3/gdk/gdkwindow.c:5071)
gdk_window_process_updates_internal (/build/buildd/gtk+2.0-2.18.3/gdk/gdkwindow.c:5233)
IA__gdk_window_process_all_updates (/build/buildd/gtk+2.0-2.18.3/gdk/gdkwindow.c:5336)
gdk_window_update_idle (/build/buildd/gtk+2.0-2.18.3/gdk/gdkwindow.c:4965)
gdk_threads_dispatch (/build/buildd/gtk+2.0-2.18.3/gdk/gdk.c:506)
g_main_dispatch (/build/buildd/glib2.0-2.22.3/glib/gmain.c:1960)
g_main_context_iterate (/build/buildd/glib2.0-2.22.3/glib/gmain.c:2591)
IA__g_main_context_iteration (/build/buildd/glib2.0-2.22.3/glib/gmain.c:2655)
nsAppShell::ProcessNextNativeEvent(int) (/home/cjones/mozilla/mozilla-central/widget/src/gtk2/nsAppShell.cpp:145)
nsBaseAppShell::DoProcessNextNativeEvent(int) (/home/cjones/mozilla/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:155)
nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, int, unsigned int) (/home/cjones/mozilla/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:293)
nsThread::ProcessNextEvent(int, int*) (/home/cjones/mozilla/mozilla-central/xpcom/threads/nsThread.cpp:510)
NS_ProcessNextEvent_P(nsIThread*, int) (/home/cjones/mozilla/ff-dbg/xpcom/build/nsThreadUtils.cpp:250)
mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (/home/cjones/mozilla/mozilla-central/ipc/glue/MessagePump.cpp:118)
MessageLoop::RunInternal() (/home/cjones/mozilla/mozilla-central/ipc/chromium/src/base/message_loop.cc:217)
MessageLoop::RunHandler() (/home/cjones/mozilla/mozilla-central/ipc/chromium/src/base/message_loop.cc:200)
MessageLoop::Run() (/home/cjones/mozilla/mozilla-central/ipc/chromium/src/base/message_loop.cc:173)
nsBaseAppShell::Run() (/home/cjones/mozilla/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:180)
nsAppStartup::Run() (/home/cjones/mozilla/mozilla-central/toolkit/components/startup/src/nsAppStartup.cpp:182)
XRE_main (/home/cjones/mozilla/mozilla-central/toolkit/xre/nsAppRunner.cpp:3551)
main (/home/cjones/mozilla/mozilla-central/browser/app/nsBrowserApp.cpp:158)
__libc_start_main (/build/buildd/eglibc-2.10.1/csu/libc-start.c:252)
_start (/build/buildd/eglibc-2.10.1/csu/../sysdeps/x86_64/elf/start.S:116)

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

No assertion failure as of http://hg.mozilla.org/mozilla-central/rev/e3e80935c165 :(.

Is this a dup of bug 544348?

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I tried applying the patch in bug 544348 but it's severely bitrotted.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #1)
> No assertion failure as of
> http://hg.mozilla.org/mozilla-central/rev/e3e80935c165 :(.
> 

... and they appear in http://hg.mozilla.org/mozilla-central/rev/f236632a9747.

Comment 4

[Joe Drew (not getting mail)]

Are we 100% sure that this is a graphics problem, and not (for example) something in our plugin code?

Comment 5

[Karl Tomlinson (:karlt)]

This is a regression from the cairo upgrade, a change in behavior of cairo_draw_with_gdk.  Maybe something was relying on behavior that wasn't promised, but something needs to be fixed.

This means that we are not fulfilling our NPAPI obligations and the spew of assertions makes it impossible to use debug m-c builds to test sites with windowless plugins.

Comment 6

[Karl Tomlinson (:karlt)]

cairo_copy_clip_rectangle_list(cairo_t*) returns the expected extents of the target surface until a clip is applied.  When the clip is applied it is not intersected with the apparent existing clip (of the target extents) because there is no clip recorded on the context, and cairo_copy_clip_rectangle_list starts returning the applied clip (even when it is outside the target extents).

This looks like a regression from
http://cgit.freedesktop.org/cairo/commit/?id=bed2701e1c89095878d549cbca8f22d84f3dda3c
where clips were disassociated from surfaces.
Before that _cairo_clip_clip would consider the target extents.

To be consistent with _cairo_gstate_clip_extents, it looks like
_cairo_clip_copy_rectangle_list should also intersect the context's clip with the target surface extents (instead of only using them when there is no clip).

With other changes I'm making to the native renderer, it may become unnecessary to fix this cause.

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: use native-rendering temp surfaces only as large as necessary

This patch was written with performance in mind but it also fixes this bug,
hence I'll attach it here.

The assertion reported here was added to check that the native renderer always
provided a clip when the target surface is smaller than the drawing (plugin) rectangle.

Since bug 574583, however, clip rectangles are not supported with Flash
Player.  But we'd still like to use small target surfaces.  With this patch
the surface size is checked directly rather than relying on the clip.

Small target surfaces mean that when a transparent plugin invalidates a small
rectangle we don't bother doing the alpha extraction over the whole plugin
rectangle.

There are also some smaller optimizations for opaque plugins here: shorter
strides between lines of drawing, and their temp surface is not cleared before
drawing.

One thing this patch doesn't do is add an expose region parameter to the
native renderer.  However, with retained layers, the clip region will often be
a good indication of the expose region.

(BTW, the reason why the bounds passed to the native renderer is that of the
plugin rectangle, not the expose rectangle, is so the renderer can use the
bounds to know the maximum extents that a plugin not supporting clips might
draw if drawing directly.)

Comment 8

[Karl Tomlinson (:karlt)]

cairo_copy_clip_rectangle_list(cairo_t*) has a much more serious bug, which we will need to fix.

If _cairo_clip_copy_rectangle_list() is passed a clip with all_clipped set (and path NULL), then it returns the gstate target extents instead of an empty clip rectangle list.

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: cairo clip fixes

This includes fixes from upstream for comment 6 and comment 8 as well as a similar fix for cairo_clip_extents().

These are all tested in the cairo test suite (again) now.

Comment 10

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/bdfeac0b9d4a

Comment 11

[Karl Tomlinson (:karlt)]

The changes in comment 7 ended up landing in bug 580440.