Closed Bug 74129 Opened 23 years ago Closed 23 years ago

Left navigation bar not repainted

Categories

(Core Graveyard :: GFX, defect, P2)

Product:
Core Graveyard
Component:
GFX
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.1

People

(Reporter: jg, Assigned: pavlov)

References

(
URL
)

Details

(Keywords: regression, testcase, top100, Whiteboard: critical for mozilla 0.9.1)

Attachments

(6 files)

partial screenshot demonstrating partial repaint of navigation area
11.66 KB, image/png
Details
Screenshot of what is happening on windows
132.92 KB, patch
Details | Diff | Splinter Review
shorten www.netscape.com testcase
305 bytes, text/html
Details
Make sure to UpdateGC() before using it
3.76 KB, patch
Details | Diff | Splinter Review
add UpdateGC() calls to the new DrawImage methods
1.90 KB, patch
Details | Diff | Splinter Review
don't use mGC in nsImageGTK
3.58 KB, patch
Details | Diff | Splinter Review 
Using Linux Mozilla cvs tip built today.

Reproducable: Always.

Steps to reproduce:
1)  Go to http://www.netscape.com/ and wait for page to load. Notice left hand
    side navigation bar with grey background.
2)  Minimise the window however your window manager does it. In Enlightenment I
    am right-clicking the titlebar.
3)  Restore the window's state (I right-click the titlebar again).

Expected Results: Page display restored
Actual Results: The side navigation table does not display the text links. The
grey background is now solid as opposed to solid with white seperating bars. The
links can still be hovered over, but the text cannot be seen on screen.
Keywords.
Keywords: mozilla0.9, regression, top100
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Suggest re-evaluation for target milestone (currently Future):

1. This is still happening in today's builds, and nicely reproducable

2. This has a serious negative effect on the Netscape Corporate Homepage - if
   you force a repaint of the browser window, the links in the left side
   navigation panel are no longer visible.


Steps to reproduce screenshot:

1) Load www.netscape.com fully (wait until throbber stops)
2) Switch virtual desktops, or do something else to cause the page to not be
   shown on screen
3) Switch back to viewing the webpage. Note that the blue-on-grey navigation
   menu is now solid grey with no text
4) Move a window over the top of the display to force a re-draw of that area

*** Bug 77651 has been marked as a duplicate of this bug. ***
*** Bug 78938 has been marked as a duplicate of this bug. ***
*** Bug 39407 has been marked as a duplicate of this bug. ***
Upping severity.  Things not drawing really sucks.  Changing platform to All
since this happens on windows and linux (and I expect mac).  I believe this is
view manager related.

Robert?  Do you have any ideas what is going on here?  There are a lot of bugs
being filed on things not drawing..  We need this fixed ASAP.
Severity: normal → critical
Keywords: mozilla0.9 → mozilla0.9.1
OS: Linux → All
Hardware: PC → All
Added screenshot posted in bug 39407 from Mike Jaques 2001-05-04 02:00
Summary: Left navigation bar not repainted → Damaged areas not repainted with new view manager
*** Bug 65107 has been marked as a duplicate of this bug. ***
Blocks: 76046
Blocks: 77544
Blocks: 73195
Keywords: mostfreq
Whiteboard: critical for mozilla 0.9.1
Target Milestone: Future → mozilla0.9.1
I'll take a look at this tomorrow.
I'm not seeing this on win32 talkback build 2001050704
I _am_ seeing this on 2001050708/Linux
This is a regression introduced between 4/22 and 4/24. A 4/22 release build on
WINNT refreshes correctly. A 4/24 release build on WINNT fails.
This bug was morphed from a Linux specific rendering bug on a specific page into
more general problem which happens on both Linux and WINNT and is easy to
reproduce. The general problem was introduced as a regression between 4/22-4/24.
The original problem is probably un-related to this regression. 
To reproduce the general problem on WINNT:

Rapidly move a command prompt window over Mozilla. Notice the chrome does not
refresh properly when the exposed area is repainted. Interestingly, Rapidly
moving a top-level window launched from Mozilla over Mozilla does not cause any
refresh problems. Switching to the old viewmanager makes the problem go away but
this behavior is a regression since builds prior to 4/22 which use the new
viewmanager work properly.
It may be unrelated to the original bug... I believe the original bug is due to
a clipping bug (remember the old clipping in the view manager that was in there
for linux?).
It would be nice if we could get a simpler testcase.
*** Bug 79496 has been marked as a duplicate of this bug. ***
The attachment I just added shows the problem with Mozilla/5.0 (X11; U; Linux
2.2.18 i686; en-US; rv:0.9) Gecko/20010505

The select and option are required to show the problem, which also seems to
matche with www.weather.com in bug 77544 (it has selects also).

Load my testcase, then minimize it and restore it, and the image disappears.
Move another window over it, and that part of the image disappears.
Shift-Reload, and the image comes back.
*** Bug 80033 has been marked as a duplicate of this bug. ***
*** Bug 79859 has been marked as a duplicate of this bug. ***
*** Bug 80154 has been marked as a duplicate of this bug. ***
*** Bug 78112 has been marked as a duplicate of this bug. ***
The general refresh problem (Not refreshing the newly exposed area when moving a
window over the top of mozilla) was created by the checkin for bug 75591. I just
did a fresh pull on WINNT and reversed the patch for bug 75591 and I no longer
see refresh problems.

The following cvs commands will undo the patch checked in for bug 75591.

cvs update -j1.275 -j1.274 mozilla/docshell/base/nsDocShell.cpp
cvs update -j1.49 -j1.48 mozilla/view/src/nsViewManager2.cpp
cvs update -j1.28 -j1.27 mozilla/view/src/nsViewManager2.h
cvs update -j3.187 -j3.186 mozilla/view/src/nsViewManager.cpp
cvs update -j3.71 -j3.70 mozilla/view/src/nsViewManager.h
cvs update -j3.50 -j3.49 mozilla/view/public/nsIViewManager.h
cvs update -j1.142 -j1.141 mozilla/layout/xul/base/src/nsBoxFrame.cpp
cvs update -j1.96 -j1.95 mozilla/layout/html/base/src/nsHTMLFrame.cpp
cvs update -j3.292 -j3.291 mozilla/layout/html/base/src/nsFrame.cpp
cvs update -j3.142 -j3.141 mozilla/layout/html/base/src/nsFrame.h

The image corrupted/not refreshed problems are probably separate problems.
Please Do not dup them to this bug.





With bug 75591's patch reversed in todays build on WINNT I do not see any
flicker when displaying preference panels. Is the patch no longer needed?. Has
it been superceded by Hyatt's paint delay?  

CC'ing the reviewer, super-reviewer for bug 75591. 
That is completely mystifying. The 75591 patch should not change anything when 
refresh is enabled.

With Hyatt's patch, 75591 is not as important, but it (or something like it) is 
still ultimately necessary so that the view manager can know what the background 
color of a document is.
I'm looking at the attachment 33621 [details]. I'm not sure if the bug Kevin's talking
about is the same as this one.

I am convinced that this is not a view manager bug. I commented out the line
>       aRenderingContext.SetClipRect(clipRect, nsClipCombine_kReplace, clipEmpty)
in nsComboboxControlFrame.cpp and the bug goes away. Note that this setting of
the clip rect is inside a PushState/PopState pair, so it shouldn't have any
effect on the rendering of the image. But it does. So I suspect a bug in gfx/GTK
clipping, possibly with some interaction with libpr0n.

I commented out the DefaultRefresh call in nsViewManager and that did NOT fix
the bug. So I'm not sure what's going on with Kevin's comment about reverting
75591, but I suspect he's looking at a different bug.
Yep. It's a gfx/GTK/libpr0n bug. Calling aContext.DrawImage invokes
nsRenderingContextImpl::DrawImage, which invokes nsImageGTK::Draw, which calls

>    copyGC = ((nsRenderingContextGTK&)aContext).GetGC();

... but without calling UpdateGC anywhere along that path. So any changes to
aContext's clip rect since the last UpdateGC are not reflected into copyGC, we
just use some random old clip rect.

Doing an UpdateGC before the GC is returned fixes the testcases.

I'll attach a patch.

This could explain a lot of clipping/drawing problems. I don't know what the
story is with Windows yet...
I'm not convinced that's a complete fix. In particular, if mAlphaPixmap is
nonnull then you cache the GC in mGC. If someone changes the clip rect and then
draws the image again, and you reuse mGC, you lose.
> Doing an UpdateGC before the GC is returned fixes the testcases.

To be more precise, it fixes the originally-reported www.netscape.com bug and
its simplified test case. I doubt it fixes the dups since many of them seem to
be reporting problems on Windows, and Windows GFX doesn't seem to have any
analogous bug to the one I'm fixing here.

By the way, here's a hint for bug diagnosis: if it only affects images, it is
almost certainly NOT view manager related. The view manager knows nothing about
images. When the view manager clips or renders incorrectly, the bugs are always
apparent with any kind of content (e.g., text only).

The general refresh problem on WIN32 created by the checkin for bug 75591 was 
caused by failing to return a status which tells the toolkit the paint was 
consumed, don't do default processing:

the following patch fixes the general refresh problem:

Index: nsViewManager.cpp
===================================================================
RCS file: /cvsroot/mozilla/view/src/nsViewManager.cpp,v
retrieving revision 3.187
diff -u -r3.187 nsViewManager.cpp
--- nsViewManager.cpp	2001/04/24 01:01:14	3.187
+++ nsViewManager.cpp	2001/05/15 01:51:52
@@ -1966,6 +1966,7 @@
 											
}
 										
}
 									}
+               	  *aStatus = nsEventStatus_eConsumeNoDefault;
 							}
 
 				break;
I filed a new bug for the general refresh problem (bug 80847).  

From this point on, this bug should only cover the original Linux refresh 
problem.
*** Bug 80554 has been marked as a duplicate of this bug. ***
Handing over to pavlov in light of the above comments.
Assignee: kmcclusk → pavlov
Status: ASSIGNED → NEW
BTW, chinerj@didntduck.org, thank you VERY much for the shortened testcase. You
have no idea how useful a good testcase is. It saved hours of trouble.
Keywords: testcase
roc: UpdateGC() gets called from inside all of the calls in 
nsRenderingContextGTK, so if the clip rect is changed, UpdateGC will be 
called.  The patch here won't really do anything.
If your patch fixes things on linux (like the bug here), then some place in 
nsRenderingContextGTK isn't calling UpdateGC()... so we should fix that instead 
of making the callers call it.

I'm pushing this to 0.9.2, but expecting to fix the drawing bugs for 0.9.1.
Summary: Damaged areas not repainted with new view manager → Left navigation bar not repainted
Target Milestone: mozilla0.9.1 → mozilla0.9.2
moving back to 0.9.1

nsRenderingContextGTK::DrawImage() (all 50 versions) need to call UpdateGC().


I'll post a new patch shortly.
Target Milestone: mozilla0.9.2 → mozilla0.9.1
ok, here is a patch that should accomplish the same thing.
looking for r= and sr=
r/sr=blizzard
Blocks: 79543
ok, fix checked in.  i'm gonna close this bug.. if any thing marked as a dup of
this isn't also fixed, please reopen the individual bug.
What about images with alpha pixmaps? From a cursory reading of the code, it 
looks like a copy of the rendering context's GC could be cached in 
nsImageGTK::mGC and then reused later, with an obsolete clip rect.
No longer blocks: 79543
I will investigate mGC in nsImageGTK before marking this bug fixed
Priority: -- → P2

    
    

    
  
-    if (IsFlagSet(nsImageUpdateFlags_kBitsChanged, mFlags)) {
-      xvalues.clip_mask = GDK_WINDOW_XWINDOW(mAlphaPixmap);
-      xvalues_mask |= GCClipMask;
-    }
+    xvalues.clip_mask = GDK_WINDOW_XWINDOW(mAlphaPixmap);
+    xvalues_mask |= GCClipMask;

Why do you remove this check?

    
    

    
  
because the GC doesn't cache the alpha mask. We have to set it everytime.

    
    

    
  
OK.  In that case r/sr=blizzard

    
    

    
  
jst r='d this the other day.

    
    

    
  
fixed
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
    

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

    
    

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

    
    

    
  
Marking verified per last comments
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

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: