Changing bit depth/# of colors busts chrome, images

VERIFIED FIXED in mozilla0.9.5

Status

defect
P3
critical
VERIFIED FIXED
20 years ago
11 years ago

People

(Reporter: elig, Assigned: kmcclusk)

Tracking

(Blocks 1 bug, {topembed})

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PDT+] ETA: Ready to checkin, Have review/super-review [from bugscape][nsbeta2-] [nsbeta3-] relnote-user)

Attachments

(1 attachment)

(Reporter)

Description

20 years ago
* TITLE/SUMMARY
Changing bit depth/# of colors busts chrome, images

* STEPS TO REPRODUCE
0) Launch Apprunner. Ideally, go to an image-intensive page.
1) Change the system bit depth (I changed from 16-bit to 8-bit on Mac OS, and 32-
bit to 16-bit on Win32).

* RESULT
 - What happened

Chrome background turns black on Mac OS, and blue toolbar items turn grey on
Win32. Page being viewed isn't dithered (or re-dithered) for the new resolution.

 - What was expected

Like Communicator 4.5, Seamonkey should be aware of system bit depth changes. I
assume this is slated for implementation, but I don't know for sure, don't see a
bug to this effect, and am thus writing this bug report up. ;)

Please note related bug #2639.

* REGRESSION

 - Occurs On
        Mac OS Apprunner (M5 optimized build)
        Win32 Apprunner (M5 optimized build [NT 4, Service Pack 3])
        (Didn't check Linux)

 - Doesn't Occur On
        Communicator 4.6 RTM for Mac OS (99118 build)


* CONFIGURATIONS TESTED

- [Mac] Power Mac 8500/120 (233 Mhz 604e), 64 MB RAM (VM on; 1 MB of VM used),
1024x768 (Thousands of Colors), Mac OS 8.6

- [Win32] Vectra VL (233 Mhz P2), 96 MB RAM, 800x600 (True Color), NT 4.0 SP3.

- [Linux] Vectra VL (266 Mhz P2), 96 MB RAM.
(Reporter)

Updated

20 years ago
QA Contact: 3853 → 1698

Updated

20 years ago
Assignee: don → kmcclusk
Component: Apprunner → Compositor
(Assignee)

Updated

20 years ago
Status: NEW → ASSIGNED
Target Milestone: M8
(Assignee)

Updated

20 years ago
Target Milestone: M8 → M10
(Reporter)

Comment 1

20 years ago
[Courtesy notice: Bug is still present in 8.16.99 AM builds; checked on Windows
NT 4.0 SP & Mac OS 8.6]
(Assignee)

Updated

20 years ago
Target Milestone: M10 → M11
(Assignee)

Comment 2

20 years ago
Movin to M11
(Assignee)

Updated

20 years ago
Target Milestone: M11 → M12
(Assignee)

Comment 3

20 years ago
Moving to M12
(Reporter)

Comment 4

20 years ago
As a courtesy note, on 9.23.99 AM M11 candidates:
	Mac OS: Chrome is busted upon resolution change; images redither properly.
	Windows: Chrome is not busted upon resolution change, but images are busted
during True color ---> 256 color resolution change.
(Assignee)

Updated

20 years ago
Assignee: kmcclusk → beard
Status: ASSIGNED → NEW
(Assignee)

Comment 5

20 years ago
I have added a new GUI event: NS_DISPLAYCHANGE. It is generated when the display
depth is changed. It is implemented on WIN32 as of Oct 6, 1999 3:06PM build.

The next step is to have the view system reload all images.

Patrick, reassigning to you.

Linux and Mac still need to generate the NS_DISPLAYCHANGE event.

Updated

20 years ago
Status: NEW → ASSIGNED

Updated

20 years ago
Target Milestone: M12 → M13

Updated

20 years ago
Target Milestone: M13 → M15
(Assignee)

Comment 6

20 years ago
*** Bug 2639 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 7

20 years ago
*** Bug 8545 has been marked as a duplicate of this bug. ***
*** Bug 29774 has been marked as a duplicate of this bug. ***

Comment 9

19 years ago
Reassigning all compositor bugs to kevin.
Assignee: beard → kmcclusk
Status: ASSIGNED → NEW
QA Contact: elig → petersen
(Reporter)

Comment 10

19 years ago
QA Assigning back to self, unless petersen feels strongly.
QA Contact: petersen → elig
(Assignee)

Comment 11

19 years ago
Moving to M16
Status: NEW → ASSIGNED
Target Milestone: M15 → M16
(Assignee)

Comment 12

19 years ago
Moving to M17
Target Milestone: M16 → M17
See also 36470.
(Assignee)

Comment 14

19 years ago
The problem on WIN32 is the nsIImages are converted to DDB's. When the depth is 
changed, we need to dump all of the existing DDB's and convert them to new DDB's 
with the current depth.

As a test I modified nsImageWin.cpp's nsImageWin :: Optimize(nsIDeviceContext* 
aContext) to set  mCanOptimize = PR_FALSE; instead of PR_TRUE.

I also added code to cause the offscreen to be reallocated to nsViewManager2.cpp

NS_IMETHODIMP nsViewManager2::DispatchEvent(nsGUIEvent *aEvent, nsEventStatus 
*aStatus)
{
	*aStatus = nsEventStatus_eIgnore;

	switch(aEvent->message)
		{

              case NS_DISPLAYCHANGED:
                  // Reset the offscreens width and height
                  // so it will be reallocated the next time it needs to
                  // draw. It needs to be reallocated because it's depth
                  // has changed.
                 *aStatus = nsEventStatus_eConsumeDoDefault;
                 mDSBounds.width = 0;
                 mDSBounds.height = 0;
      break;

This fixes the problem for the most part. I still have to go to edit preferences 
in mozilla and press OK, then expose the window to get it to refresh the Images 
properly. Editing the preferences causes a style change reflow.

This is not a long term solution since it requires storing all images as DIB's 
which probably more inefficient than storing them as DDB's.
(Assignee)

Comment 15

19 years ago
In an attempt to provide a more general solution I tried destroying all of the 
frames and flushing the image cache when the display depth changed:
This should remove all of the nsIImages instances and cause them to be 
recreated with the correct depth:

NS_IMETHODIMP
PresShell::HandleEvent(nsIView         *aView,
                       nsGUIEvent*     aEvent,
                       nsEventStatus*  aEventStatus,
                       PRBool&         aHandled)
{
  void*     clientData;
  nsIFrame* frame;
  nsresult  rv = NS_OK;
  
  NS_ASSERTION(!(nsnull == aView), "null view");

  if (aEvent->message == NS_DISPLAYCHANGED) {
    nsCOMPtr<nsIImageManager> imageManager;
    nsresult result;
    imageManager = do_GetService(kImageManagerCID, &result);
    
    if (NS_FAILED(result)) {
      NS_ASSERTION(PR_FALSE, "Can not get image manager");
    } else {
      imageManager->FlushCache();
    }
    ReconstructFrames();
    aHandled = PR_TRUE;
    *aEventStatus = nsEventStatus_eConsumeDoDefault;
    return NS_OK;
  }

Unfortunately, it does not seem to fix the problem at all. The content area 
completely disappears after the depth change, and clicking the url bar causing a 
crash because the DocShell as a null window pointer. Not sure yet, why this 
happens. It should be safe to reframe.
*** Bug 41444 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 17

19 years ago
This bug is marked "future" because it is not critical for RTM (Release To 
Manufacturing). If anyone believes it is critical, please explain why in
this bug. 
Target Milestone: M17 → Future
If this is not fixed by FCS, we must mention it in the release notes...
Keywords: relnote
Note: dependent bug 36470 is nsbeta2+, so perhaps this should be too?
No longer blocks: 36470
Keywords: relnotensbeta2
(Assignee)

Comment 20

19 years ago
Bug 36470 should not be dependant on this bug.

This bug is concerned with what do we do when a NS_DISPLAYDEPTH_CHANGED event is 
generated as the result of the user changing the display depth dynamically.

The solution is to destroy all of the images at the old depth and create new 
ones at the new depth, which will be very inefficent, but that OK because it 
seldom happens, and it happens in response to a user action.

Bug 36470 is the result of using multiple monitors on the Mac. It is 
probably related to palette problems, but fixing this bug will have no effect on 
bug 36470 because NS_DISPLAYDEPTH_CHANGED will never be generated.

By the way Navigator 4.x does not handle display depth changes gracefully 
either.

In my opion this should be nsBeta2-.

Comment 21

19 years ago
Putting on [nsbeta2-] radar. Not critical to beta2.  Adding "nsbeta3" keyword 
for consideration of a fix for that milestone. 

Keywords: nsbeta3
Whiteboard: [nsbeta2-]
(Assignee)

Comment 22

19 years ago
Adding correctness keyword
Keywords: correctness
(Assignee)

Comment 23

19 years ago
Work around is to restart Mozilla after display depth change.
Whiteboard: [nsbeta2-] → [nsbeta2-] [nsbeta3-]
(Assignee)

Comment 24

19 years ago
Marking nsbeta3-
I'll take QA if no-one objects, since Eli is elsewhere.

4xp, because 4.x does *so* handle depth changes gracefully; also added 
relnoteRTM.
Keywords: 4xp, relnoteRTM
QA Contact: elig → mpt
Whiteboard: [nsbeta2-] [nsbeta3-] → [nsbeta2-] [nsbeta3-] relnote-user

Comment 26

19 years ago
*** Bug 56425 has been marked as a duplicate of this bug. ***

Updated

19 years ago
Keywords: mozilla0.8mozilla0.9

Comment 27

18 years ago
Using the code from bug 6061 (on 6061 it will fix for Macintosh system changes 
only) and mixing it with Kevin's attempt to use the NS_DISPLAYCHANGED handler 
seems to work resetting the chrome/images:


include "nsIChromeRegistry.h"
...
NS_IMETHODIMP
PresShell::HandleEvent(nsIView         *aView,
                       nsGUIEvent*     aEvent,
                       nsEventStatus*  aEventStatus,
                       PRBool&         aHandled)
{
  void*     clientData;
  nsIFrame* frame;
  nsresult  rv = NS_OK;
  
  NS_ASSERTION(!(nsnull == aView), "null view");

  if (aEvent->message == NS_DISPLAYCHANGED) {

    nsCOMPtr<nsIChromeRegistry> 
        chromeReg(do_GetService("@mozilla.org/chrome/chrome-registry;1"));
    if (!chromeReg) {
        throw NS_ERROR_FAILURE;
    } else {
        chromeReg->RefreshSkins();
    }
  }
   

Updated

18 years ago
Blocks: 67625

Updated

18 years ago
Keywords: nsbeta2, nsbeta3patch, review

Comment 28

18 years ago
*** Bug 85062 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 29

18 years ago
Clearing the image cache to refresh images in the content area of the page:

From pavlov:

#include "imgICache.h"

nsCOMPtr<imgICache> imgCache(do_GetService("@mozilla.org/image/cache;1"));
imgCache->ClearCache(TRUE); // chrome images
imgCache->ClearCache(FALSE); // everything 'cept chrome images

This won't effect images currently on the screen though.  You would need to walk
through all of the <html:img> and <xul:image> elements and force them to reload
the image. 
What about the view manager's backing store?
(Assignee)

Comment 31

18 years ago
Yes that would have to be reallocated as well as any other offscreen images.

Added topembed keyword.
Keywords: topembed

Updated

18 years ago
Whiteboard: [nsbeta2-] [nsbeta3-] relnote-user → [from bugscape][nsbeta2-] [nsbeta3-] relnote-user

Comment 32

18 years ago
raising severity per embedding customer request
Severity: normal → critical
Keywords: nsbranch
Note that we'll have to hook up an Apple Event to get notified of display manager 
changes on Mac OS.
(Assignee)

Comment 35

18 years ago
Simply forcing the backbuffer to be reallocated fixes the problem on WIN32. 

I tested switching back and forth between 256 (Indexed color), true color, and
16bit color and it works fine with this patch.

The new imagelib seems to have taken care of the other issues I was seeing back
on 5/24/2000.  The new imagelib keeps all images as 32bit and coverts when drawing.

The backbuffer needed to be reallocated because all images are blitted into it
before going to the screen. If the backbuffer has a smaller depth then the
screen (Which was the case when going from 256 (Indexed color) to true color)
color information was lost when the images were blitted to the backbuffer.
So how do we get the NS_DISPLAYCHANGED notification to the view manager on other 
platforms?
(Assignee)

Comment 37

18 years ago
n Mac it we should add a method to nsWindow which is called when the display has
changed:

PRBool nsWindow::ReportDisplayChange()
{
	// nsEvent
	nsGUIEvent changeEvent;
	changeEvent.eventStructType = NS_GUI_EVENT;
	changeEvent.message = NS_DISPLAYCHANGED;
	changeEvent.time = PR_IntervalNow();

	// nsGUIEvent
	changeEvent.widget		= this;
	changeEvent.nativeMsg		= nsnull;

	// dispatch event
	return (DispatchWindowEvent(changeEvent));
}

The call to DisplayWindowEvent will take care of getting the message to the
viewmanager.
(Assignee)

Comment 38

18 years ago
GTK needs the same method. It also has a DispatchWindowEvent method which
already passes the XP events up to the viewmanager.
(Assignee)

Comment 39

18 years ago
I split the Mac and GTK work to generate the NS_DISPLAYCHANGE into separate bugs:

Mac  bug 101843
GTK  bug 101844



(Assignee)

Updated

18 years ago
Whiteboard: [from bugscape][nsbeta2-] [nsbeta3-] relnote-user → Waiting for review/super-review [from bugscape][nsbeta2-] [nsbeta3-] relnote-user

Comment 40

18 years ago
Comment on attachment 50978 [details] [diff] [review]
fix bug by forcing the backbuffer to be re-allocated

r=karnaze
Attachment #50978 - Flags: review+

Comment 41

18 years ago
Removing myself from cc.
Kevin should we nsbranch+ this one? It looks like an old, but interesting bug.
(Assignee)

Comment 43

18 years ago
Marking nsbranch+
Keywords: nsbranchnsbranch+

Comment 44

18 years ago
Comment on attachment 50978 [details] [diff] [review]
fix bug by forcing the backbuffer to be re-allocated

sr=attinasi
Attachment #50978 - Flags: superreview+
Wait a hookey minute. This shouldn't go in until we've got fixes for all 
platforms.

Comment 46

18 years ago
This bug does not to be done in eMojo timeframe (but it's needed soon by our
embedding customer).
(Assignee)

Comment 47

18 years ago
"Wait a hookey minute. This shouldn't go in until we've got fixes for all 
platforms."

The patch covers the XP portion of fixing the problem. The platform specific
work is done under separate bugs and it will be easier to develop the platform
specific work if the XP portion is already checked in.

It just happens that the WIN32 platform specific code is already in the tree so
fixing the XP portion fixes it for WIN32.


(Assignee)

Updated

18 years ago
Whiteboard: Waiting for review/super-review [from bugscape][nsbeta2-] [nsbeta3-] relnote-user → ETA: Ready to checkin, Have review/super-review [from bugscape][nsbeta2-] [nsbeta3-] relnote-user
(Assignee)

Comment 48

18 years ago
Note: The bug does NOT apply to Linux. It isn't possible to dynamically change
the display depth. I closed out bug 101844 as invalid.

bug 101843 covers the Mac platform specific work.




check it in - PDT+
Whiteboard: ETA: Ready to checkin, Have review/super-review [from bugscape][nsbeta2-] [nsbeta3-] relnote-user → [PDT+] ETA: Ready to checkin, Have review/super-review [from bugscape][nsbeta2-] [nsbeta3-] relnote-user
(Assignee)

Comment 50

18 years ago
checked into the trunk and MOZILLA_0_9_4_BRANCH.

Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
Target Milestone: Future → mozilla0.9.5

Comment 51

18 years ago
i wonder if it could apply to QtEmbedded

Comment 52

18 years ago
wow, an old bug.

mpt - you're the qa contact; will you be able to verify this bug?  If not,
please reassign qa contact to me.  Thanks.

Updated

18 years ago
Blocks: 101843
I can't verify this since it doesn't fix the originally reported user-visible
bug. Changing QA.
QA Contact: mpt → lchiang
(Assignee)

Comment 54

18 years ago
The reporter indicated both Mac and WIN32 were failing. I fixed it on WIN32 and
filed a separate bug for the Mac (bug 101843). We should be able verify this bug
is fixed on WIN32. 

Updated

18 years ago
QA Contact: lchiang → tpreston

Comment 55

18 years ago
Verified fixed win xp 2001111503 and win 2k build 2001111403
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.