Closed Bug 324803 Opened 18 years ago Closed 18 years ago

drawWindow on canvas fails on cairo-enabled builds

Categories

(Core :: Graphics, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mossop, Assigned: vlad)

References

Details

(Whiteboard: cairo)

Attachments

(3 files, 1 obsolete file)

My extension (Tab Sidebar) uses drawWindow to display a preview of the web page. That is currently not drawing anything on cairo builds.

The testcase in bug 313462 comment 13 also fails to draw anything. Other canvas examples do work though.
Whiteboard: cairo
The same with Tab Preview.
nsThebesRenderingContext::Init
NS_ERROR("Should never be called.");
return NS_ERROR_NOT_IMPLEMENTED;

Call stack:
nsThebesRenderingContext::Init
nsThebesDeviceContext::CreateRenderingContext
nsLayoutUtils::CreateOffscreenContext
PresShell::RenderOffscreen
nsCanvasRenderingContext2D::DrawWindow
This will probably continue to fail until a thebes-backed canvas impl is landed (shortly after linux switches to thebes rendering).
Attached patch fixSplinter Review
This is just some code cribbed from PresShell::RenderOffscreen and remixed with Thebes goodness.

With this patch, drawWindow works OK, but text does not display correctly. Since everything else does, I blame the text stack. Interestingly, results vary depending on the orientation of the text ... when drawWindow is being rotated, text sometimes draws nearly correctly.
Assignee: nobody → roc
Status: NEW → ASSIGNED
Attachment #214629 - Flags: superreview?(vladimir)
Attachment #214629 - Flags: review?(vladimir)
Maybe I did a bad build, but when I tested with this patch applied, my extension's call to drawWindow made firefox use 99% CPU and become unusable.
The testcase in bug 313462 does not hang the browser, though it also doesnt draw anything like it does with non-cairo builds. Both my extension (Tab Sidebar) and someone elses (Tab Preview) hang whenever they use drawWindow.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060313 Firefox/1.6a1
Hmm. Could you get a stack track of the hung process?
Ran this in a debug build and got the following two assertions. I'm not sure how to get a stack trace but if you point me in the right direction then I can give it a try.

###!!! ASSERTION: No DC: 'aDC', file c:/mozilla/source/HEAD/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp, line 489
cairo_win32_scaled_font_select_font:SelectObject: The handle is invalid.
###!!! ASSERTION: No DC: 'aDC', file c:/mozilla/source/HEAD/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp, line 621
cairo_win32_scaled_font_select_font:SelectObject: The handle is invalid.
Aha.

I think the problem is that we're creating an nsThebesRenderingContext for a cairo image surface, and expecting gfxWindowsFonts to work with it, and it doesn't of course.

So I guess this is going to be hard to fix before we land the big canvas changeover to Thebes :-(.
Comment on attachment 214629 [details] [diff] [review]
fix

Hopefully Vlad can use this code when/after his canvas changes land.
Attachment #214629 - Flags: superreview?(vladimir)
Attachment #214629 - Flags: review?(vladimir)
Depends on: 334512
Blocks: 334512
No longer depends on: 334512
Blocks: 334728
No longer blocks: 334512
Flags: blocking1.9a1?
Attached patch fix for trunk (obsolete) — Splinter Review
Fix for the trunk; there are a few problems still (some images don't seem to be scaled correctly, I think it's tiled images), but those can be fixed in followup patches.  This also happens to fix bug 333672, since it was sitting in my buffer.
Assignee: roc → vladimir
Attachment #219814 - Flags: review?(roc)
Trying to copy state from the mCairo to a new gfxContext seems fragile to me. In particular, as written, this seems to lose the clip. Either we need to wrap mCairo in a gfxContext or we need the canvas to use gfxContext directly; I was assuming that the latter is going to happen anyway.

But won't this patch hit exactly the problem that I did before, that gfxWindowsFonts won't work with the image surface?
Crap, the clip is a problem.  I'll figure something out for that.  Canvas on the trunk will eventually move to using thebes natively; this code though is going to go on the 1.8 branch as well (trying to get canvas in sync between trunk and branch, with image encoders and stuff moving to branch also).  The gfxContext switch will happen after that on the trunk.

I don't want to expose a gfxContext constructor that takes a cairo_t* though, since we want to eventually push cairo down below thebes and not export any of its symbols.

Text does work though; canvas now creates a real windows surface (DIB) for its backing store.  It looks nice, too, since we get windows rendering small-size fonts with AA and stuff directly instead of having cairo scale down without filtering.

I'll work on fixing the clipping.
Blocks: 315207
Also see Bug 352488 on this.
(In reply to comment #15)
> Also see Bug 352488 on this.
> 

That bug is certianly annoying.  This particular bug seems to be fixed now though.
> That bug is certianly annoying.  This particular bug seems to be fixed now
> though.

I disagree. drawWindow still appears to fail on chrome windows.
Flags: blocking1.9a1? → blocking1.9+
Attachment #219814 - Attachment is obsolete: true
Attachment #219814 - Flags: review?(roc)
The text fast path wasn't updating the CTM for the font, so if the font happened to stick around, we would end up using the wrong thing and cause misrendering of text in one place or the other.
Attachment #243497 - Flags: review?(pavlov)
The patch in comment 18 appears to nicely fix bug 358086 for me.

However now I can see things more clearly I can report an additional issue to that of comment 17. Textboxes (multiline and single line) are just transparent blocks, allowing whatever colour is underneath the canvas element to show through. Any text in the textbox appears to get some kind of transform to its colour, here my text is black in the webpage but in the canvas which has yellow behind it the textbox appears yellow and the text a bright purple. Interestingly as I increase the canvas size (hence reducing the scaling) the textbox suddenly becomes light pink in background and the text remains purple.

I've attached a screenshot to hopefully make this more clear.
Comment on attachment 243497 [details] [diff] [review]
fix drawWindow breaking text rendering due to CTM changing

r= with the matrix thing we talked about
Attachment #243497 - Flags: review?(pavlov) → review+
Checked in; please file a new bug for the widget issue, though I think that's caused by windows native theme code not respecting alpha (or rather, us expecting it to respect alpha).
No longer blocks: 352488
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: