Closed Bug 380115 Opened 14 years ago Closed 13 years ago

Linux 16-bit widget size issues (black rectangle at bottom of long page)

Categories

(Core :: Graphics, defect, P2)

x86
Linux
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: schapel, Assigned: roc)

References

()

Details

(Keywords: regression)

Attachments

(5 files, 1 obsolete file)

Reproducible: Always

Steps to Reproduce:
1. Go to the URL given and scroll to the bottom of the page.

There's a large black rectangle obscuring the text at the bottom of the page. I can reproduce in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070508 Minefield/3.0a5pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070507 SeaMonkey/1.5a. I can certainly search for the regression range and/or try to make a reduced test case if this isn't an obvious duplicate.
Keywords: regression
The bug also occurs in SeaMonkey 2007050701 on Linux.
Steve, if you can find the regression range that would be great.  Thanks.
OS: Windows XP → All
Regression Range:
The black rectangle does not appear in Firefox trunk build 2006122204
The black rectangle does     appear in Firefox trunk build 2006122304
Assignee: general → nobody
Blocks: 364742
Component: GFX → GFX: Thebes
Flags: blocking1.9?
QA Contact: ian → thebes
Assignee: nobody → vladimir
Flags: blocking1.9? → blocking1.9+
Attached file testcase
The content area here is getting its own widget, because the div has an overflow: auto style on it.  This widget is 26000 pixels too high, and we're trying to draw to it from its top left, thus overflowing the 16-bit signed coordinate space we have to work with in cairo.

Attached is a simple testcase.
Priority: -- → P1
Target Milestone: --- → mozilla1.9beta1
Attached patch patch (obsolete) — Splinter Review
So here's a patch that fixes this.  This switches cairo to 24.8 instead of 16.16.  Note that pixman is still in 16.16 mode; anything that requires fallback that's "big" will most likely be misrendered.  I think that's ok (and shouldn't affect, say, Quartz).  Also this updates the constant for rect conditioning.
Attachment #275664 - Flags: superreview?(roc)
Attachment #275664 - Flags: review?(pavlov)
Comment on attachment 275664 [details] [diff] [review]
patch

This is not the right patch.
Attachment #275664 - Attachment is obsolete: true
Attachment #275664 - Flags: superreview?(roc)
Attachment #275664 - Flags: review?(pavlov)
Attached patch the real patchSplinter Review
The real patch.
Attachment #275839 - Flags: superreview?(roc)
Attachment #275839 - Flags: review?
Comment on attachment 275839 [details] [diff] [review]
the real patch

comment the definition of CAIRO_COORD_MAX to indicate where that magic number comes from
Attachment #275839 - Flags: superreview?(roc) → superreview+
Comment on attachment 275839 [details] [diff] [review]
the real patch

please file a follow up bug to clamp in the X surface?
Attachment #275839 - Flags: review?(pavlov) → review+
The data URL in the URL field does weird things on linux with this patch applied; I'm not sure where the problem is.

Old original real-world testcase URL:

http://apcmag.com/6070/mozilla_ceo_speaks_out_on_the_future_of_firefox_the_complete_8_000_wor
vlad: whats going on with this bug?
Priority: P1 → P2
Target Milestone: mozilla1.9 M8 → ---
This gets fixed, on at least win32, by the followup patch to bug 384681.  I think it should get fixed on at least OSX; linux -might- get fixed, but I think we have 16 bit issues on linux that are deeper than cairo itself.
Depends on: 384681
Yeah, this is fixed on win32 and mac.  Still need to check linux.
So I just checked linux; everything works fine for me.  But a few other folks (dholbert and others on irc) checked, and they have issues near the bottom of that page.  He was running ubuntu gutsy with the intel driver; I'm running feisty (older) with the intel driver.  I'm not sure what's going on here.
We're doing ok on all platforms as long as the main page is the large one, as in the test case, or even a page with 32k lines consisting of "abcd<br>" in a 36pt font (resulting page is 2.5M pixels high, we render things fine).

However, the apcmag testcase has a div with 'overflow-x: auto; overflow-y: auto;'; adding that to my long testcase causes breakage on linux.
Duplicate of this bug: 403929
Sorry, I didn't find this bug before, I found out, that it is the same problem and the differences are just some configure options.

(cairo/xulrunner built with or without glitz support)
So this is the first part of a fix -- the old pre-cairo code seemed to just go ahead and pass large values to gtk/gdk, which most likely clamped them internally.  Since we need to create a surface that knows nothing about gdk, we just clamp these widhts/heights to max xlib 16-bit values.

With that patch, stuff starts rendering, but there are still problems -- in particular, the "long testcase" is still half-broken.  If you scroll to the bottom, stuff doesn't update, and if you scroll too quickly sometimes updating stops.  If you scroll back to the top and then start scrolling down then things will render.

We should probably take this initial bit no matter what, but it'll need more work for the final fix.
Attachment #303117 - Flags: review?(roc)
Comment on attachment 303117 [details] [diff] [review]
first part of a fix

Use a helper method to clamp mSize so we don't have to duplicate this code...
Attachment #303117 - Flags: review?(roc) → review+
Michael, could you take this on?  Figuring out what's going beyond that initial bit is probably a few days' worth of work deep in gdk and gtk2 widget code, and I know nothing about that area...
Summary: Black rectangle at the bottom of long page → Linux 16-bit widget size issues (black rectangle at bottom of long page)
(In reply to comment #20)
> Michael, could you take this on?  Figuring out what's going beyond that initial
> bit is probably a few days' worth of work deep in gdk and gtk2 widget code, and
> I know nothing about that area...
> 

What part of gtk/gdk does this touch? It only seems like Cairo touches Xlib to me...
(I only wish I understood the code more myself :( )
Cairo itself does, but the underlying problem here is in the gtk2 widget code -- I have no idea how gtk2 handled this case in firefox 2, where a child widget was > 65536 in height.
In Firefox 2 did we do the special casing for that size or did GTK do that for us? I'd like to know whereabouts I should be searching...
I have no idea :(  From a quick glance at the code, I don't see anything in nsWindow.cpp that clamped things to 64k, so I have no clue how it worked.
Optimistically bouncing this over to roc, based on irc conversation.
Assignee: vladimir → roc
I think we need to fix this, though I don't know how often it will actually come up in real-world browsing.  It might not be common enough?
Flags: blocking1.9+
(In reply to comment #26)
> I think we need to fix this, though I don't know how often it will actually
> come up in real-world browsing.  It might not be common enough?

Of course I don't know about others, but I run into this problem about once a week.
(And it is not one page that I visit on a regular basis.)
Flags: tracking1.9+
Attached patch fixSplinter Review
This fixes it using the wonderful gdk_window_get_internal_paint_info API.
Attachment #309253 - Flags: review?(vladimir)
Whiteboard: [needs review]
Comment on attachment 309253 [details] [diff] [review]
fix

That's way too easy, cheater. ;)
Attachment #309253 - Flags: review?(vladimir) → review+
Whiteboard: [needs review] → [needs landing]
Checked in
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Flags: in-testsuite?
This isn't testable with the infrastructure currently available.
Sure, but eventually we should return to it and do so when that's possible.  If we were to just flat-out ignore it completely, then it'd be -.
I was looking at bug 425394, and the reduction brings me to something like attachment 292672 [details].

I've updated the testcase slightly in attachment 312128 [details]. See bug 425394 comment 4 how to reproduce the issue.

I also tested that testcase on Windows, and the lines are not drawn any more after line 574. Do you happen to know if there's a bug filed for that?
Is it bug 215055?
yes, looks like it. Thanks for the pointer.
You need to log in before you can comment on or make changes to this bug.