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)
Core Graveyard
GFX
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)
(deleted),
patch
|
karnaze
:
review+
attinasi
:
superreview+
|
Details | Diff | Splinter Review |
* 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•25 years ago
|
QA Contact: 3853 → 1698
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M8
Assignee | ||
Updated•25 years ago
|
Target Milestone: M8 → M10
Reporter | ||
Comment 1•25 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•25 years ago
|
Target Milestone: M10 → M11
Assignee | ||
Comment 2•25 years ago
|
||
Movin to M11
Assignee | ||
Updated•25 years ago
|
Target Milestone: M11 → M12
Assignee | ||
Comment 3•25 years ago
|
||
Moving to M12
Reporter | ||
Comment 4•25 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•25 years ago
|
Assignee: kmcclusk → beard
Status: ASSIGNED → NEW
Assignee | ||
Comment 5•25 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•25 years ago
|
Status: NEW → ASSIGNED
Updated•25 years ago
|
Target Milestone: M12 → M13
Updated•25 years ago
|
Target Milestone: M13 → M15
Comment 9•25 years ago
|
||
Reassigning all compositor bugs to kevin.
Assignee: beard → kmcclusk
Status: ASSIGNED → NEW
QA Contact: elig → petersen
Reporter | ||
Comment 10•25 years ago
|
||
QA Assigning back to self, unless petersen feels strongly.
QA Contact: petersen → elig
Comment 13•24 years ago
|
||
See also 36470.
Assignee | ||
Comment 14•24 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•24 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•24 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
Comment 18•24 years ago
|
||
If this is not fixed by FCS, we must mention it in the release notes...
Keywords: relnote
Comment 19•24 years ago
|
||
Note: dependent bug 36470 is nsbeta2+, so perhaps this should be too?
Assignee | ||
Comment 20•24 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•24 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 23•24 years ago
|
||
Work around is to restart Mozilla after display depth change.
Whiteboard: [nsbeta2-] → [nsbeta2-] [nsbeta3-]
Assignee | ||
Comment 24•24 years ago
|
||
Marking nsbeta3-
Comment 25•24 years ago
|
||
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
Updated•24 years ago
|
Whiteboard: [nsbeta2-] [nsbeta3-] → [nsbeta2-] [nsbeta3-] relnote-user
Comment 26•24 years ago
|
||
*** Bug 56425 has been marked as a duplicate of this bug. ***
Updated•24 years ago
|
Keywords: mozilla0.8
Updated•24 years ago
|
Keywords: mozilla0.8 → mozilla0.9
Comment 27•24 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();
}
}
Comment 28•23 years ago
|
||
*** Bug 85062 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 29•23 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.
Comment 30•23 years ago
|
||
What about the view manager's backing store?
Assignee | ||
Comment 31•23 years ago
|
||
Yes that would have to be reallocated as well as any other offscreen images.
Added topembed keyword.
Keywords: topembed
Updated•23 years ago
|
Whiteboard: [nsbeta2-] [nsbeta3-] relnote-user → [from bugscape][nsbeta2-] [nsbeta3-] relnote-user
Comment 32•23 years ago
|
||
raising severity per embedding customer request
Severity: normal → critical
Keywords: nsbranch
Comment 33•23 years ago
|
||
Note that we'll have to hook up an Apple Event to get notified of display manager
changes on Mac OS.
Assignee | ||
Comment 34•23 years ago
|
||
Assignee | ||
Comment 35•23 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.
Comment 36•23 years ago
|
||
So how do we get the NS_DISPLAYCHANGED notification to the view manager on other
platforms?
Assignee | ||
Comment 37•23 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•23 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•23 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•23 years ago
|
Whiteboard: [from bugscape][nsbeta2-] [nsbeta3-] relnote-user → Waiting for review/super-review [from bugscape][nsbeta2-] [nsbeta3-] relnote-user
Comment 40•23 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•23 years ago
|
||
Removing myself from cc.
Comment 42•23 years ago
|
||
Kevin should we nsbranch+ this one? It looks like an old, but interesting bug.
Assignee | ||
Comment 43•23 years ago
|
||
Marking nsbranch+
Comment 44•23 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+
Comment 45•23 years ago
|
||
Wait a hookey minute. This shouldn't go in until we've got fixes for all
platforms.
Comment 46•23 years ago
|
||
This bug does not to be done in eMojo timeframe (but it's needed soon by our
embedding customer).
Assignee | ||
Comment 47•23 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•23 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•23 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.
Comment 49•23 years ago
|
||
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•23 years ago
|
||
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
Comment 51•23 years ago
|
||
i wonder if it could apply to QtEmbedded
Comment 52•23 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.
Comment 53•23 years ago
|
||
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•23 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•23 years ago
|
QA Contact: lchiang → tpreston
Comment 55•23 years ago
|
||
Verified fixed win xp 2001111503 and win 2k build 2001111403
Status: RESOLVED → VERIFIED
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•