invisible views are painted

RESOLVED FIXED in mozilla1.0.1

Status

()

Core
XUL
P4
normal
RESOLVED FIXED
18 years ago
10 years ago

People

(Reporter: Tomi Leppikangas, Assigned: David Hyatt)

Tracking

({perf})

Trunk
mozilla1.0.1
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

18 years ago
When selecting text with paintflash on, area where Edit->Copy menuitem
should be, flashes. This happend even that menuitem is not shown
on screen (menu is not drop down).

Things to reproduce:

First open Edit menu and close it. Then put paint flashing on and select some
text from mozilla. You see that selection flashes, and then flashes area below
edit menu, just where 'Copy' menuitem should be. 

This same happens for 'Go' menu items 'Forward' and 'Back' when clickin on
back/forward buttons, but that is bit harder to see.

I have small patch that fixes this, but i am not sure is this checking
for visibility at right place?
(Reporter)

Comment 1

18 years ago
Created attachment 7233 [details] [diff] [review]
small fix to nsViewManager2::ProcessPendingUpdates

Comment 2

18 years ago
Adding perf keyword
Keywords: perf
Great job tracking that down, tomi :-), but I think the fix needs to be placed 
in the IsRectVisible. This will prevent the invisible views from being put in 
the queue to be processed altogether. Can you try adding the following lines to 
the top of IsRectVisible?

 aView->GetVisibility(visibility);
 if(nsViewVisibility_kHide == visibility) {
   return PR_FALSE;
 }

If not, I'll give it a try after I've integrated some other view manager changes 
and see if it clears up the problem.
Thanks.
Status: NEW → ASSIGNED
Target Milestone: --- → M16
(Reporter)

Comment 4

18 years ago
Yes it works ok. Actually there is that test in nsViewManager2::UpdateView
but it is after we add region to dirty region. Maybe moving that test
to begining of function does the job?
(Reporter)

Comment 5

18 years ago
I found other similar situation:

Start mozilla, put paint flash on, mouse over menubar. First
time you mouse over menu, it gets loaded and build. And when building
menu, menuview gets resized and that causes repaint -> you see
flash at 0.0. 
Actuly you see two flashed, one from menu-view, other from parent-view.

This flash is seen just at first mouseover,  others mouseovers
doest generae menus.

I attach patch that fixed this and first bug.
(Reporter)

Comment 6

18 years ago
Created attachment 7481 [details] [diff] [review]
patch #2, fixed both bugs
I checked in a fix for the first problem since I already had other view changes 
in my tree and had tested it on multiple platforms. I'll look at the second case 
next.
Fixed in 4/12/2000 4:29 PM build.
Thanks for tracking this down and providing the patch Tomi!  :-)

-- Kevin
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Reporter)

Comment 9

18 years ago
Looks good, marking as verified.
Status: RESOLVED → VERIFIED
(Reporter)

Updated

18 years ago
Status: VERIFIED → REOPENED
Component: Views → XP Toolkit/Widgets: Menus
Resolution: FIXED → ---
(Reporter)

Comment 10

18 years ago
Looks like this has come back with scrolling menus? Didnt
see any changes in view code, i assign this to menus/evaughan@netscape.com
(Reporter)

Comment 11

18 years ago
evaughan i reassing this to you, you changed menus to use scrolling
views?
Assignee: kmcclusk → evaughan
Status: REOPENED → NEW

Comment 12

18 years ago
Moving all bugs that are not dogfood+, nsbeta2+,features, or nsbeta2- to M21
Target Milestone: M16 → M21

Comment 13

18 years ago
fixed
Status: NEW → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 14

18 years ago
I'm seeing this on a 6/14 cvs pull built on solaris/native.  Steps to reproduce:

  * start mozilla
  * turn on paint flashing (prefs->debug, flashing, debug window, capslock, etc)
  * pop down edit menu - notice while doing this that the first time a menu
      title is moused over a region the size of the exposed menu will flash
      at the display's 0,0
  * get rid of edit menu
  * select some text on the webpage - select region will flash, along with an
      area right where the edit->copy menu entry would be
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 15

18 years ago
Pav this only happens on linux. Can you take a look
Assignee: evaughan → pavlov
Status: REOPENED → NEW

Comment 16

18 years ago
Is this due to the paste menu item being changed constantly whenever the edit
menu is used or the clipboard stuff is used?  reassigning to editor... this
might also be xpapps, or layout, or menus.. sigh.  i'm not sure who should own
this.  pink?
Assignee: pavlov → beppe
Component: XP Toolkit/Widgets: Menus → Editor
QA Contact: petersen → sujay
(Reporter)

Comment 17

18 years ago
I bet this is views thing. I have patch for this now. Patch walks trought
parent views and see are they all visible, if not, dont paint.

(Reporter)

Comment 18

18 years ago
Created attachment 12005 [details] [diff] [review]
patch #3 for views

Comment 19

18 years ago
assigning to Kin to review patch
Assignee: beppe → kin
Target Milestone: M21 → M18

Comment 20

18 years ago
adding patch keyword
Keywords: patch
(Reporter)

Comment 21

18 years ago
Something between builds 07/28/2000 08:00 and 07/28/2000 21:00
changes so hidden menus flicker every time when mousing over
menubar. This make significant slowdown on linux.

This is show when turning paintflash on and mousing over menubar,
menu area flash even menu is hidden. Also seen in editor, toolbar buttons shift
slightly when mousing over menubar.

Comment 23

18 years ago
Reassigning to kmcclusk@netscape.com (the owner of the view module) so he can
evaluate the nsViewManager2 submitted patch.
Assignee: kin → kmcclusk
I can not apply Tomi's patch. It is legal for child views to be visible and have 
hidden ancestors. This is required to support CSS's specification of the 
visibility property.
Well, there's a difference between being visible from a progromatic standpoint
and being physcially mapped on screen, right?
In this case they are the same. It is perfectly legal for a child view to render  
 even though it has an ancestor that is hidden. Setting the visibility to hidden 
on an ancestor does not affect whether a child view is rendered. It was designed 
this way to support CSS's visibility property.  If you need a view and all of 
it's children to be hidden you need to walk the view sub-tree and mark all of 
the view's hidden. 

Reassigning back to kin. Adding myself to the CC list.

Assignee: kmcclusk → kin

Comment 27

18 years ago
This isn't really an editor issue. Changing Component to "XP Toolkit/Widgets: 

Menus"

Assignee: kin → pinkerton
Component: Editor → XP Toolkit/Widgets: Menus
QA Contact: sujay → jrgm
what's the actual bug  here?
Assignee: pinkerton → trudelle
(Reporter)

Comment 29

18 years ago
Actual bug is, or how it shows, from tor's comment:

  * start mozilla
  * turn on paint flashing (prefs->debug, flashing, debug window, capslock, etc)
  * pop down edit menu - notice while doing this that the first time a menu
      title is moused over a region the size of the exposed menu will flash
      at the display's 0,0
  * get rid of edit menu
  * select some text on the webpage - select region will flash, along with an
      area right where the edit->copy menu entry would be

Actual performance bug here is that views that are visible, but are child
of invisible view, are painted on screen. Nothing is show thougt native parent
window is invisible, but trip to x-server is done anyway, and this is slow on
linux. My patch adds loop that goest trought all parent and make sure they
are all visible, and if not, don't repaint.

This isn't so big issue with edit menu, but there might be situations
where this helps more.
why is this a toolkit bug and not a view bug?
Well there was an assumption that this was a view bug, but this really isn't a 
bug in the view system. It behaves this way by design (i.e child views 
visibility is independent of their ancestor's visibility) So if the desired 
behavior is pop a menu up and down without causing the painting of the child 
views, the code which manages the menus will need to walk the view hierarchy and 
set all of the child view's visibility. 

In most cases, when CSS is used child elements inherit there visibility from 
their parent and this issue does not come up. I believe the menu code does not 
use CSS properties to control the visibility of the popup. Instead, the menu 
popup code directly manipulate the visibility of a parent view. If this is the 
case, then it's up to the menu code to set the visibility on the child views.

Comment 32

18 years ago
reassigned to pinkerton, ccpavlov, ->M20, but could still take patch if it is
worth the risk.
Assignee: trudelle → pinkerton
Priority: P3 → P4
Target Milestone: M18 → M20

Comment 33

18 years ago
->future
Target Milestone: M20 → Future
I don't see this problem with my view manager in Win32, but I don't understand 
why you would see it with nsViewManager2 either.

When the hidden view is invalidated, we cause an invalidation to be sent to a 
hidden widget. Surely this should just be dropped on the floor either by the X 
server or (better) by our X widget code?
Target Milestone: Future → mozilla1.0
new owner
Assignee: pinkerton → hyatt
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla1.0.1

Comment 36

16 years ago
Any news on that one?
Markus, can you verify that this is still a bug? I think it may have been fixed
by my fix for bug 130263.

Comment 38

16 years ago
seems okay

Comment 39

16 years ago
Can we marke this fixed or is there still something that needs to be done?
It's been fixed, at least the problem that Tomi originally reported.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago16 years ago
Resolution: --- → FIXED

Updated

10 years ago
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.