Jesse Ruderman Reporter
	Description • 20 years ago

Steps to reproduce: Open about six galleries worth of porn images at once using
the "linked images" bookmarklet.

Result: Firefox stops repainting its interface and content area, with the
exception that it still paints parts of the content area when the content area
is scrolled.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050305
Firefox/1.0+.  I'm using Windows XP.  My video card is NVIDIA GeForce4 MX 4000.

This bug is similar to bug 184933, which was fixed by a patch in bug 205893. It
returned because the patch to that bug was effectively backed out for a
performance gain in bug 284716.

	ArronM (:paper) Assignee
	Comment 1 • 20 years ago

Thanks.  I have a patch for this.  When Moz runs out of GDI objects, we don't
check the results CreateCompatibleBitmap when we make a Drawing Surface.  It
fails, and Moz stops painting.  A check to see if it fails, and using
CreateDIBSection if so, fixes this problem.

Patch soon to come.  I'm looking for any other places we create a DDB without
first checking.

	ArronM (:paper) Assignee
	Comment 3 • 20 years ago

Comment on attachment 176455 [details] [diff] [review]
Do DDB surface. Do DIB if fails.

Ere, you are familiar with this code, care to do a review?

	Stuart Parmenter
	Comment 4 • 20 years ago

The patch looks right to me.  If ere doesn't review shortly I will.

	Ere Maijala (slow)
	Comment 5 • 20 years ago

Comment on attachment 176455 [details] [diff] [review]
Do DDB surface. Do DIB if fails.

r=emaijala provided you fix the comment (Create a DDB -> Create a DIB) and
remove the prinf from nsImageWin.cpp. 

Are you sure this will let us get away with consuming all resources without
causing other problems? I'm not confident this is a good thing.

	Boris Zbarsky [:bzbarsky]
	Comment 6 • 20 years ago

Er... what's with the printf in non-debug code?

	Ere Maijala (slow)
	Comment 7 • 20 years ago

Something I wanted to be removed to get r+ (although I made a typo).

	ArronM (:paper) Assignee
	Comment 8 • 20 years ago

Comment on attachment 176455 [details] [diff] [review]
Do DDB surface. Do DIB if fails.

Woah, nsImageWin wasn't supposed to be in the patch.  Consider it (and thus the
printf) removed.

	ArronM (:paper) Assignee
	Comment 9 • 20 years ago

Well, it's never a good idea to use up all resources.  But there is no way to
determine when we use them up.  It isn't linked to # of GDI objects.  I can make
Moz run out of GDI resources by opening 4 images from my digicam, which are 6MP
each (ie. large), so usage seems dependent on size.  But the limit may be set by
OS/BIOS/Video Card/Video Driver, or maybe it's hardcoded into windows.  Only the
goats know.

It also appears that in Win2k (and hopefully XP), each application gets its own
limit.  When Moz hits it's limit, other apps still painted for me.  But, I have
my doubts that that is true.  The other apps could just be using DIBs when DDBs
run out too!

	ArronM (:paper) Assignee
	Comment 10 • 20 years ago

Minor nits changed:
-Removal of printf/nsImageWin
-"Create a DDB if we need Pixel Access" changed to "Create a DIB.."
-Exit early if binfo is nsnull

Unless ere or bz objects, I'm assuming the r+ can be transfered from the
obsolete patch, so I'm only asking for a SR.

	Boris Zbarsky [:bzbarsky]
	Comment 11 • 20 years ago

Comment on attachment 176528 [details] [diff] [review]
Create DDB surface, if failed, try DIB

sr=bzbarsky

	ArronM (:paper) Assignee
	Comment 12 • 20 years ago

Checking in nsDrawingSurfaceWin.cpp;
/cvsroot/mozilla/gfx/src/windows/nsDrawingSurfaceWin.cpp,v  <-- 
nsDrawingSurfaceWin.cpp
new revision: 3.21; previous revision: 3.20
done

	Ashley Bischoff (blog at handcoding.com)
	Comment 13 • 20 years ago

(In reply to comment #12)
> Checking in nsDrawingSurfaceWin.cpp;

So, -> FIXED?

	ArronM (:paper) Assignee
	Comment 14 • 20 years ago

(In reply to comment #13)
> So, -> FIXED?

Won't know until people try the latest nightly.  Fixes it for me, but I may not
have tested it as aggressively as others.

	Alex Kapranoff
	Comment 15 • 20 years ago

BTW, something like this happens in StaticGray mode on X Window. Look here
https://bugzilla.mozilla.org/show_bug.cgi?id=117310

The misbehaviour is exactly the same -- some parts of UI and pages are not
redrawn after loading many images (but only in StaticGray mode).

	ArronM (:paper) Assignee
	Comment 16 • 20 years ago

Status?  Does FF/Moz stop repainting in the latest nightly builds?

	Ere Maijala (slow)
	Comment 17 • 20 years ago

I haven't tried a build that showed the problem, but at least I couldn't
reproduce on WinXP SP 2 and the latest SeaMonkey nightly.

	Bernard Alleysson
	Comment 18 • 20 years ago

(In reply to comment #16)
Windows 2000 SP4, firefox trunk 20050310, Radeon 7500 64MB:

1) Loading a lot of jpeg (about 10 windows x 10 tabs x 20 images)
2) GDI object count at 2865 for firefox

Results: 
* firefox has some major problems with fonts (just try "about:cache" for instance)
fonts on web sites (most of them) are wrong, ie you get bold fonts, fonts with
the wrong sizes, ...
* other applications suffer from rendering problem (bitmaps, fonts), for
instance try IE

Remarks:
Closing a few windows seems to fix the rendering bug, the critical GDI object
count value seems to be 2500 for me

	ArronM (:paper) Assignee
	Comment 19 • 20 years ago

darn, so it's the fonts now.  I was eventually able to reproduce #18, although
it took a long long long time.  Once I got it, Win2k went into spasms of "not
enough virtual memeory" (even though I had 300M of real memory free) and other
apps had a problem drawing.  Everything was horribly slow.

I'm considering writing a failsafe routine that dumps ALL Mozilla DDBs when we
run out of resources.  Considering that running out of resources is a pretty
major matter for a computer, I think such a bold reaction is acceptable.  Plus,
it wouldn't take much overhead (compared to maintaining a handle count). At
least it'll be something until someone works magic on Bug 218861.

	Aaron Kaluszka
	Comment 20 • 20 years ago

Accidental reassign

	Bernard Alleysson
	Comment 21 • 20 years ago

(In reply to comment #19)
From my testing (Windows 2000), I can see that applications affect each other
with regard to GDI resources. You can't prevent an application from taking all
availables resources. Firefox can run out of GDI handles no matter the effort
you put into limiting its use of GDI resources. So I think that it's ok to use
DDB and fallback to DIB after all (I had to open more than 2000 images to defeat
it !).

I can see that GDI resources are released by IE when windows are closed but GDI
handles are not released by Firefox. They are retained by the memory cache
instead (I get the same as IE when the Firefox memory cache is disabled). So
clearing the inactive memory cache content (to release GDI handles) when the
creation of any DDB fails could be considered (I guess that disabling the memory
cache is not an option).

	Boris Zbarsky [:bzbarsky]
	Comment 22 • 20 years ago

One interesting thing may be to have a "gdi pressure" observer (like our
memory-pressure observer) mechanism.  If ddb creation fails, fire the
notification (which should probably flush our image and font caches), and retry...

	ArronM (:paper) Assignee
	Comment 23 • 20 years ago

This patch is very similar to Bug 205893, Attachment 127547 [details] [diff] ("patch to fix
problem").  Since that patch was r- by stuart, I'm going to ask for trouble and
ask stuart to review this one.	The difference is that my patch does less. 
It's not a rotating cache, rather it's a simple failsafe that flushes all image
DDBs if we are unable to create a new DDB.  I consider this a crisis state, as
the OS is not able to even properly paint its low-resources warning message.  A
retry to optimize the image is not done for the same reason (the OS is in a
crisis state), however, new images will try to optimize.

As much as I like the idea of flushing the cache, I think keeping it simple, at
least for the 1.8 release, is best.  If we were to go the 'flushing the cache'
route, I'd rather just un-optimize when no open pages are referencing the
image, and re-optimize when it's pulled from memory cache and used again.  Even
that plan, though, would require the failsafe presented in this patch (for
those who open a crazy amount of images), or a similar failsafe mechanism.

	Jesse Ruderman Reporter
	Comment 24 • 20 years ago

Following the steps in comment 0 no longer makes Firefox stop repainting.  This
bug is still open because of related problems first mentioned in comment 18.

	HopeSeekr of xMule
	Comment 25 • 20 years ago

Using 20050312 I *still* get this bug, or maybe a new one.  It happens after x
refreshes per minute of pages.  I can't figure it out but it is fairly low, say
25 refreshes in a minute?  Attached is a screenshot (gmail.jpg).  It happens to
every page in every window and tab and refreshing does not solve the problem,
although you can view the blocked images individually fine.

	Bernard Alleysson
	Comment 27 • 20 years ago

(In reply to comment #24)
> This bug is still open because of related problems first mentioned in comment
> 18.

I can confirm that it's certainly not fixed for me (Windows 2000). After some
browsing, suddenly other applications stop repainting (WMP, iTunes, ...). Even
the Start menu won't work (it doesn't expand, clicking Start does nothing)! The
solution: close all firefox windows. Each time I take a look at the GDI object
count and it is between (1500 - 2500) for firefox.exe, even if all windows are
blank. I have 1GB RAM and the memory cache is set to 32768 KB.

	Bernard Alleysson
	Comment 28 • 20 years ago

setting blocking flags:
Once the memory cache is filled up with images, Firefox takes almost all GDI
resources and other applications stop repainting.

	Steve England [:stevee]
	Comment 29 • 20 years ago

Bernard, try reducing your memory cache size. Once the cache is full, firefox
should start releasing its oldest cached (GDI) objects so new objects can be
cached. If you set your cache too big, then I think you're more likely to hit
the OS limit of GDI objects, which can cause problems.

	Bernard Alleysson
	Comment 30 • 20 years ago

(In reply to comment #29)
> Bernard, try reducing your memory cache size.
I tried and it works for me, indeed. I even tried to disable the memory cache.
Unfortunately some sites don't load when the memory cache is disabled (!). The
lowest working value for me is 1KB.
 
> Once the cache is full, firefox
> should start releasing its oldest cached (GDI) objects so new objects can be
> cached.
Once the cache is full, old objects are replaced by new ones but once the memory
cache is full, it stays full, and GDI resources don't go down.

> If you set your cache too big, then I think you're more likely to hit
> the OS limit of GDI objects, which can cause problems.
The default for my setup (1GB RAM) is 31744 KB, and I've set it up to 32768 KB
because I've tried to change the size, and it's easier to remember.

	Bernard Alleysson
	Comment 31 • 20 years ago

Just a thought, is it possible (about:config) not to cache any images (only css
+ html)? Given the performance improvement of using DDB, maybe caching images in
the memory cache is not necessary anymore.

	Steve England [:stevee]
	Comment 32 • 20 years ago

> Once the cache is full, old objects are replaced by new ones but once the memory
> cache is full, it stays full, and GDI resources don't go down.
But that's expected behaviour right? Once the memory cache is full, as new items
are cached, old items fall out the cache, so the number of GDI objects and
memory used should stay around constant. Even when you close all but one tab,
the cache remains full, handles to GDI Objects still remain.

If you choose to clear your cache then you will notice that GDI Objects are
released. As expected.

	Boris Zbarsky [:bzbarsky]
	Comment 33 • 20 years ago

> Given the performance improvement of using DDB, maybe caching images in
> the memory cache is not necessary anymore.

DDB isn't exactly universally used across the platforms we support....

	Bernard Alleysson
	Comment 34 • 20 years ago

fwiw, the problem I've run into with memory cache size set to 0 is bug 184304

	Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
	Comment 35 • 20 years ago

Bug 290510 has a testcase, assuming this is the same issue.

	ArronM (:paper) Assignee
	Comment 36 • 20 years ago

Work-In-Progress, but I thought I'd attach it as it stands since it's in a
working state.	I'd like to throw in a GetFreeSystemResources32 in
nsGdiFailsafe::IsOkayToOptimize() check for Win9x, and browse through the patch
once more before asking for a review.

This patch requires Attachment #181941 [details] [diff] of Bug 292051

Quick Summary:
- Use GDI resources until there are no more

- Set a total image DDB memory limit to 1/2 of what we are currently using
(memory limit probably dependent upon graphic card), and clear all GDI
resources asynchronously (flushing all takes too long time to do synchronously)


- When memory limit is reached, and an image wants to be optimized, flush
enough DDBs to make it happen.

- If the surface creation routine runs out of memory, flush image DDBs and try
again.	Surface is more important than images.


Via Bug 292051, DDBs will also be flushed based on time, making it less likely
we will ever reach the limit.

In case I didn't mention it, GDI resources are limited in number on
Win9x/ME/NT, AND limited in total memory size on ALL platforms.

	ArronM (:paper) Assignee
	Comment 37 • 19 years ago

*** Bug 290510 has been marked as a duplicate of this bug. ***

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 38 • 19 years ago

why not make the voidarray a concrete member, rather than a pointer member?

	Daniel Veditz [:dveditz]
	Comment 39 • 19 years ago

Checked attachment 176528 [details] [diff] [review] into the aviary and mozilla1.7 branches.

	Asa Dotzler [:asa]
	Comment 40 • 19 years ago

transferring nomination to 1.8b4. If we don't get it there, we don't get it for 1.1

	Asa Dotzler [:asa]
	Comment 41 • 19 years ago

It would be great if we could get a fix for the next release but I don't think
we'll hold if one isn't available in time.  I'm certainly not trying to
discourage further work here.

	Stefan Ring
	Comment 42 • 19 years ago

For me the regression happens in the changes made from nsImageWin.cpp, revision
3.139 to 3.140. On Win2K/XP, this effectively cancels out my patch from #205893
which has been in use for nearly 18 months. I just noticed a few days ago when I
finally upgraded from Mozilla 1.7.7 to 1.7.11. Note that it wasn't a self
fulfilling prophecy - I noticed the problems FIRST and looked into bonsai
BECAUSE OF the problems.

	Boris Zbarsky [:bzbarsky]
	Comment 43 • 19 years ago

Right.  From bug 284716 comment 0:

  Apparently DIBs use less GDI resources.  However, DDBs are much
  much faster than DIBs. 

So it sounds like we're more likely to run out of GDI resources now that we're
using DDBs.  We're also not taking the pretty severe perf hit we were taking
before (order of > 400% on a real-life site in bug 277762, for example).

	Stefan Ring
	Comment 44 • 19 years ago

Ok so you are right, I forgot about the performance aspect. I didn't notice any
difference back then, however. Now I remember having seen one page using lots of
transparent images that was indeed a lot slower. But it seemed to be the
transparency - didn't notice performance issues with opaque images.

However, I'm sorry about my previous comment which may have been a bit biased
because I thought of my patch as a one size fits all permanent cure which it
doesn't seem to be. I still prefer slower, reliable painting to faster graphics
operations producing display errors, though.

	HyperHacker
	Comment 45 • 19 years ago

(In reply to comment #18)
> fonts on web sites (most of them) are wrong, ie you get bold fonts, fonts with
> the wrong sizes, ...
> * other applications suffer from rendering problem (bitmaps, fonts), for
> instance try IE
This happens any time Windows runs out of graphic resources, whether it's caused by Firefox or not. The most likely reason is that it doesn't have enough free resources to load the correct font, so it simply chooses a 'fall-back' font which is always loaded (usually Fixedsys). Similarly, I've seen cases where list boxes will be drawn at position 0,0 instead of their real position. Unfortunately I don't think much, if anything, can be done to prevent this; loading a lot of images uses up a lot of resources, and using up a lot of resources makes Windows act funny.

Also, it's important to note that number of images or number of GDI objects have very little relation to the problem; the real issue is the amount of resources these objects take up. If you load 10000 1x1 black-and-white images, you've allocated several thousand GDI objects, but only a few K of video memory. On the other hand, if you load a single 5000x5000 24-bit colour image, you've only allocated a few GDI objects, but nearly 100MB of video memory.

	Craig
	Comment 46 • 19 years ago

(In reply to comment #45)

> Unfortunately I don't think much, if anything, can be done to prevent this;
> loading a lot of images uses up a lot of resources, and using up a lot of
> resources makes Windows act funny.

When other browsers use 1/5 the GDI resources that Mozilla does, it is clear that Mozilla is being extremely badly behaved and needs to be fixed. Clearly something can be done, if everyone else except Mozilla is managing to do it better.

	Justin Kerk
	Comment 47 • 17 years ago

Fixed by bug 366548?

	Steve Chapel
	Comment 48 • 17 years ago

(In reply to comment #46)
> When other browsers use 1/5 the GDI resources that Mozilla does...

Can you provide steps to reproduce with a recent nightly builds of Firefox 3 (after October 3, 2007)?

	Steve Chapel
	Comment 49 • 17 years ago

Opening <http://a2pg.com/testcase/images.php?images=2000> causes Firefox 3 beta 1 to use more than 4000 GDI objects (temporarily). In comparison, opening the same page in IE 7 makes it use no more than 700 GDI objects at any time. Even with Firefox's use of many times more GDI objects, I can't reproduce any repainting problem, though.

	Wayne Mery (:wsmwk)
	Comment 50 • 17 years ago

Last patch is comment 39.
closing FIXED per comment 49 and WFM testing Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b4pre) Gecko/2008021504