Closed Bug 6061 Opened 25 years ago Closed 23 years ago

Changing bit depth/# of colors busts chrome, images

Categories

(Core Graveyard :: GFX, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.5

People

(Reporter: elig, Assigned: kmcclusk)

References

(Blocks 1 open bug)

Details

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

Attachments

(1 file)

* 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.
QA Contact: 3853 → 1698
Assignee: don → kmcclusk
Component: Apprunner → Compositor
Status: NEW → ASSIGNED
Target Milestone: M8
Target Milestone: M8 → M10
[Courtesy notice: Bug is still present in 8.16.99 AM builds; checked on Windows NT 4.0 SP & Mac OS 8.6]
Target Milestone: M10 → M11
Movin to M11
Target Milestone: M11 → M12
Moving to M12
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: kmcclusk → beard
Status: ASSIGNED → NEW
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.
Status: NEW → ASSIGNED
Target Milestone: M12 → M13
Target Milestone: M13 → M15
*** Bug 2639 has been marked as a duplicate of this bug. ***
*** Bug 8545 has been marked as a duplicate of this bug. ***
*** Bug 29774 has been marked as a duplicate of this bug. ***
Reassigning all compositor bugs to kevin.
Assignee: beard → kmcclusk
Status: ASSIGNED → NEW
QA Contact: elig → petersen
QA Assigning back to self, unless petersen feels strongly.
QA Contact: petersen → elig
Moving to M16
Status: NEW → ASSIGNED
Target Milestone: M15 → M16
Moving to M17
Target Milestone: M16 → M17
See also 36470.
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.
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. ***
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
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-.
Putting on [nsbeta2-] radar. Not critical to beta2. Adding "nsbeta3" keyword for consideration of a fix for that milestone.
Keywords: nsbeta3
Whiteboard: [nsbeta2-]
Adding correctness keyword
Keywords: correctness
Work around is to restart Mozilla after display depth change.
Whiteboard: [nsbeta2-] → [nsbeta2-] [nsbeta3-]
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
*** Bug 56425 has been marked as a duplicate of this bug. ***
Keywords: mozilla0.8mozilla0.9
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(); } }
Blocks: 67625
Keywords: nsbeta2, nsbeta3patch, review
*** Bug 85062 has been marked as a duplicate of this bug. ***
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?
Yes that would have to be reallocated as well as any other offscreen images. Added topembed keyword.
Keywords: topembed
Whiteboard: [nsbeta2-] [nsbeta3-] relnote-user → [from bugscape][nsbeta2-] [nsbeta3-] relnote-user
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.
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?
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.
GTK needs the same method. It also has a DispatchWindowEvent method which already passes the XP events up to the viewmanager.
I split the Mac and GTK work to generate the NS_DISPLAYCHANGE into separate bugs: Mac bug 101843 GTK bug 101844
Whiteboard: [from bugscape][nsbeta2-] [nsbeta3-] relnote-user → Waiting for review/super-review [from bugscape][nsbeta2-] [nsbeta3-] relnote-user
Comment on attachment 50978 [details] [diff] [review] fix bug by forcing the backbuffer to be re-allocated r=karnaze
Attachment #50978 - Flags: review+
Removing myself from cc.
Kevin should we nsbranch+ this one? It looks like an old, but interesting bug.
Marking nsbranch+
Keywords: nsbranchnsbranch+
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.
This bug does not to be done in eMojo timeframe (but it's needed soon by our embedding customer).
"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.
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
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
checked into the trunk and MOZILLA_0_9_4_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Target Milestone: Future → mozilla0.9.5
i wonder if it could apply to QtEmbedded
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.
Blocks: 101843
I can't verify this since it doesn't fix the originally reported user-visible bug. Changing QA.
QA Contact: mpt → lchiang
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.
QA Contact: lchiang → tpreston
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.

Attachment

General

Created:
Updated:
Size: