Closed
Bug 3289
Opened 26 years ago
Closed 25 years ago
[PP] BLOCK toolbar buttons eat CPU on mouseover
Categories
(Core :: Layout, defect, P1)
Tracking
()
VERIFIED
WORKSFORME
M15
People
(Reporter: akkzilla, Assigned: hyatt)
References
()
Details
(Whiteboard: [Perf])
Mouse over the toolbar buttons on the Linux apprunner. Notice that mouseover
updating gets way behind.
Updated•26 years ago
|
Assignee: karnaze → evaughan
Updated•26 years ago
|
Assignee: evaughan → mcafee
Comment 1•26 years ago
|
||
When you mouse over the toolbar buttons the attribute "pseudoclass" is bieng set
to hover. This causes a reflow. But 2 problems occur on Linix. The reflow seems
to be slow and no redraw occurs. I stepped through this on the debugger and a
redraw does happen. But for some reason linix does not redraw the component.
Comment 2•26 years ago
|
||
I also traced through it with some printf's and the mouse events are coming in at
normal speed and we are reporting the correct frame hit. I think the right things
in terms of event handling and css are happening quickly, but the redraw is
somehow broken.
Adding myself to the cc list.
Comment 3•26 years ago
|
||
Is this only happening on Linux?
Comment 4•26 years ago
|
||
Is this the same bug as the "have to click twice on buttons to get them to work"
bug? If so, that would seem to be a problem with events and my previous comment
just suggested that events work fine. Hrm.
Comment 5•26 years ago
|
||
yes, only on linux.
Updated•26 years ago
|
Priority: P2 → P1
Summary: [PP] toolbar buttons eat CPU on mouseover → [PP BLOCK] toolbar buttons eat CPU on mouseover
Comment 6•26 years ago
|
||
Akkana thinks this is a blocker for the milestone, so it sounds like its higher
priority than landing an improved cut & paste.
Updated•26 years ago
|
Target Milestone: M3
Comment 7•26 years ago
|
||
set target M3
Comment 8•26 years ago
|
||
With my limited and primitive linux debugging techniques, I've found that the
button is noticing the style change, triggers a reflow, the reflow happens, all
is good, but nsTitledButtonFrame::Paint() never gets called. It does on Mac. It's
like it's doing all the work but never displaying it.
Adding michaelp to the cc list, he may have a clue.
i'm adding ramiro since he's nominally the owner for the gtk gfx/widget
implementations. i know that refreshes under gtk seem painfully slow and this
is high on the list of things to address. we also know that image rendering is
*really* bad, so images definitely add to the pain (this is also high on the
list of things to fix). ramiro/chris, if the area of a single button is damaged
what's the damage repair area come back as through a paint message?
Comment 10•26 years ago
|
||
The problem here is that doing a mouse over creates 3 Invalidate calls for the
entire window area as seen:
nsWidget::Invalidate({x=0,y=0,w=616,h=626}, 0)
nsWidget::Invalidate({x=0,y=0,w=612,h=437}, 0)
nsWidget::Invalidate({x=0,y=0,w=374,h=21}, 0)
and upon moving the mouse away from the button:
nsWidget::Invalidate({x=0,y=0,w=616,h=626}, 0)
nsWidget::Invalidate({x=0,y=0,w=612,h=437}, 0)
nsWidget::Invalidate({x=0,y=0,w=374,h=21}, 0)
nsWidget::Invalidate({x=0,y=0,w=616,h=626}, 0)
nsWidget::Invalidate({x=0,y=0,w=612,h=437}, 0)
nsWidget::Invalidate({x=0,y=0,w=374,h=21}, 0)
in turn flooding the GTK drawing queue with redraw messages. There is no reason
we should be redrawing everything whenever we want to highlight/unhighlight a
button. Ramiro says that the same thing is happening under windows, you just
arn't seeing it.
Comment 11•26 years ago
|
||
The problem here is that doing a mouse over creates 3 Invalidate calls for the
entire window area as seen:
nsWidget::Invalidate({x=0,y=0,w=616,h=626}, 0)
nsWidget::Invalidate({x=0,y=0,w=612,h=437}, 0)
nsWidget::Invalidate({x=0,y=0,w=374,h=21}, 0)
and upon moving the mouse away from the button:
nsWidget::Invalidate({x=0,y=0,w=616,h=626}, 0)
nsWidget::Invalidate({x=0,y=0,w=612,h=437}, 0)
nsWidget::Invalidate({x=0,y=0,w=374,h=21}, 0)
nsWidget::Invalidate({x=0,y=0,w=616,h=626}, 0)
nsWidget::Invalidate({x=0,y=0,w=612,h=437}, 0)
nsWidget::Invalidate({x=0,y=0,w=374,h=21}, 0)
in turn flooding the GTK drawing queue with redraw messages. There is no reason
we should be redrawing everything whenever we want to highlight/unhighlight a
button. Ramiro says that the same thing is happening under windows, you just
arn't seeing it.
Comment 12•26 years ago
|
||
It appears to work for me today, albeit slowly. Looks like something got better,
though I did just upgrade to gtk1.2. I can see the rollover feedback and the
buttons depress when I click on them.
Comment 13•26 years ago
|
||
FYI, on Mac also the entire window is redrawn when you mouse over a button. To
see this, you have to build with double buffering turned off (nsViewManager.cpp,
uncomment #define NO_DOUBLE_BUFFER).
Comment 14•26 years ago
|
||
doing the NO_DOUBLE_BUFFER thing on Linux is Really Painful to watch :(
Comment 15•26 years ago
|
||
This should be assigned to one of the Gecko folks as a performance issue. The
window should not be completely redrawn when a button is reflowed. If the button
does not change size only the button should be redrawn. If it does then its
container should be redrawn and on up. Worst case the window is redraw but this
should be rare.
Updated•26 years ago
|
Priority: P1 → P2
Comment 16•26 years ago
|
||
setting p1.Should fix this for M3, but won't hold for it.
Comment 17•26 years ago
|
||
wrong comment, should be p2
Comment 18•26 years ago
|
||
See also 3762. That bug describes specifically the redraw on mouseover problem,
and is XP. Troy has it right now.
Comment 19•26 years ago
|
||
Style changes are resulting in document reflows unecessarily.
CC-ing kipp, peterl.
#0 nsViewManager::UpdateView (this=0x80efae8, aView=0x8176dd8,
aRect=@0xbfffc990, aUpdateFlags=4) at nsViewManager.cpp:1462
#1 0x40d09328 in nsViewManager::ResizeView (this=0x80efae8, aView=0x81b2ce0,
width=7154, height=402) at nsViewManager.cpp:1950
#2 0x403fa7bc in nsFrame::DidReflow (this=0x8195a30, aPresContext=@0x80a5418,
aStatus=1) at nsFrame.cpp:994
#3 0x404ca412 in nsFormControlFrame::DidReflow (this=0x8195a30,
aPresContext=@0x80a5418, aStatus=1) at nsFormControlFrame.cpp:194
#4 0x403f75ad in nsContainerFrame::DidReflow (this=0x81954f8,
aPresContext=@0x80a5418, aStatus=1) at nsContainerFrame.cpp:103
#5 0x403ea946 in nsAreaFrame::DidReflow (this=0x81954f8,
aPresContext=@0x80a5418, aStatus=1) at nsAreaFrame.cpp:317
...
#12 0x40552ce5 in nsToolboxFrame::Reflow (this=0x817e938,
aPresContext=@0x80a5418, aDesiredSize=@0xbfffce10,
aReflowState=@0xbfffce54, aStatus=@0xbfffcf40) at nsToolboxFrame.cpp:358
#13 0x4040ec53 in nsInlineReflow::ReflowFrame (this=0xbfffd00c,
aFrame=0x817e938, aIsAdjacentWithTop=1, aReflowStatus=@0xbfffcf40)
at nsInlineReflow.cpp:316
#14 0x403f035e in nsBlockFrame::ReflowInlineFrame (this=0x817da08,
aState=@0xbfffd7b0, aLine=0x819e778, aFrame=0x817e938,
aKeepLineGoing=0xbfffcf84) at nsBlockFrame.cpp:2650
#15 0x403eeae2 in nsBlockFrame::ReflowLine (this=0x817da08,
aState=@0xbfffd7b0, aLine=0x819e778, aKeepReflowGoing=0xbfffcfd4)
at nsBlockFrame.cpp:1816
#16 0x403ee3b5 in nsBlockFrame::ReflowDirtyLines (this=0x817da08,
aState=@0xbfffd7b0) at nsBlockFrame.cpp:1564
#17 0x403ed5e1 in nsBlockFrame::Reflow (this=0x817da08,
aPresContext=@0x80a5418, aMetrics=@0xbfffdca0, aReflowState=@0xbfffdab4,
aStatus=@0xbfffdc4c) at nsBlockFrame.cpp:984
#18 0x403f4ee0 in nsBlockReflowContext::ReflowBlock (this=0xbfffdc5c,
aFrame=0x817da08, aSpace=@0xbfffdce4, aIsAdjacentWithTop=1,
aComputedOffsets=@0xbfffdc3c, aFrameReflowStatus=@0xbfffdc4c)
at nsBlockReflowContext.cpp:153
...
#28 0x403f7e7d in nsContainerFrame::ReflowChild (this=0x8131df0,
aKidFrame=0x8175b68, aPresContext=@0x80a5418, aDesiredSize=@0xbfffedd0,
aReflowState=@0xbfffed28, aStatus=@0xbfffee54) at nsContainerFrame.cpp:371
#29 0x40427b81 in ViewportFrame::Reflow (this=0x8131df0,
aPresContext=@0x80a5418, aDesiredSize=@0xbfffef40,
aReflowState=@0xbfffee58, aStatus=@0xbfffee54) at nsViewportFrame.cpp:432
#30 0x40403206 in nsHTMLReflowCommand::Dispatch (this=0x81be718,
aPresContext=@0x80a5418, aDesiredSize=@0xbfffef40, aMaxSize=@0xbfffef30,
aRendContext=@0x820edc0) at nsHTMLReflowCommand.cpp:165
#31 0x40418708 in PresShell::ProcessReflowCommands (this=0x80f3870)
at nsPresShell.cpp:1178
#32 0x40416f0b in PresShell::ExitReflowLock (this=0x80f3870)
at nsPresShell.cpp:623
#33 0x4041978d in PresShell::AttributeChanged (this=0x80f3870,
aDocument=0x80a3780, aContent=0x81998f8, aAttribute=0x80bb5d0, aHint=-1)
at nsPresShell.cpp:1590
#34 0x40cc7690 in XULDocumentImpl::AttributeChanged (this=0x80a3780,
aChild=0x81998f8, aAttribute=0x80bb5d0, aHint=-1) at nsXULDocument.cpp:1409
#35 0x40caa1aa in RDFElementImpl::SetAttribute (this=0x81998e8,
aNameSpaceID=0, aName=0x80bb5d0, aValue=@0xbffff0d4, aNotify=1)
at nsRDFElement.cpp:1729
#36 0x404da1e4 in nsButtonFrameRenderer::SetPseudoClassAttribute (
this=0x819d280, value=@0xbffff128, notify=1)
at nsButtonFrameRenderer.cpp:77
#37 0x404da837 in nsButtonFrameRenderer::ToggleClass (this=0x819d280,
aValue=0, c=@0xbffff15c, notify=1) at nsButtonFrameRenderer.cpp:177
#38 0x404da2c2 in nsButtonFrameRenderer::SetActive (this=0x819d280, aActive=0,
notify=1) at nsButtonFrameRenderer.cpp:95
#39 0x404dadd5 in nsButtonFrameRenderer::HandleEvent (this=0x819d280,
aPresContext=@0x80a5418, aEvent=0xbffff218, aEventStatus=@0xbffff244)
at nsButtonFrameRenderer.cpp:286
#40 0x40551d00 in nsTitledButtonFrame::HandleEvent (this=0x819d1c8,
aPresContext=@0x80a5418, aEvent=0xbffff218, aEventStatus=@0xbffff244)
at nsTitledButtonFrame.cpp:1000
#41 0x403e7a81 in nsEventStateManager::GenerateMouseEnterExit (this=0x8201978,
aPresContext=@0x80a5418, aEvent=0xbffff590) at nsEventStateManager.cpp:283
#42 0x403e7532 in nsEventStateManager::PreHandleEvent (this=0x8201978,
aPresContext=@0x80a5418, aEvent=0xbffff590, aTargetFrame=0x81770b0,
aStatus=@0xbffff518) at nsEventStateManager.cpp:98
#43 0x4041a406 in PresShell::HandleEvent (this=0x80f3870, aView=0x8176dd8,
aEvent=0xbffff590, aEventStatus=@0xbffff518) at nsPresShell.cpp:1923
#44 0x40d02280 in nsView::HandleEvent (this=0x8176dd8, event=0xbffff590,
aEventFlags=8, aStatus=@0xbffff518) at nsView.cpp:824
#45 0x40d021f9 in nsView::HandleEvent (this=0x8175c80, event=0xbffff590,
aEventFlags=8, aStatus=@0xbffff518) at nsView.cpp:806
#46 0x40d021f9 in nsView::HandleEvent (this=0x8175bd0, event=0xbffff590,
aEventFlags=8, aStatus=@0xbffff518) at nsView.cpp:806
#47 0x40d04dd2 in nsScrollingView::HandleEvent (this=0x8175bd0,
aEvent=0xbffff590, aEventFlags=8, aStatus=@0xbffff518)
at nsScrollingView.cpp:874
#48 0x40d021f9 in nsView::HandleEvent (this=0x80efcd8, event=0xbffff590,
aEventFlags=28, aStatus=@0xbffff518) at nsView.cpp:806
#49 0x40d08b6c in nsViewManager::DispatchEvent (this=0x80efae8,
aEvent=0xbffff590, aStatus=@0xbffff518) at nsViewManager.cpp:1707
#50 0x40d00378 in HandleEvent (aEvent=0xbffff590) at nsView.cpp:63
#51 0x40086d56 in nsWidget::DispatchEvent (this=0x8175cf0, event=0xbffff590,
aStatus=@0xbffff554) at nsWidget.cpp:818
#52 0x40086c34 in nsWidget::DispatchWindowEvent (this=0x8175cf0,
event=0xbffff590) at nsWidget.cpp:778
#53 0x40086e0c in nsWidget::DispatchMouseEvent (this=0x8175cf0,
aEvent=@0xbffff590) at nsWidget.cpp:844
#54 0x40083e6b in handle_motion_notify_event (w=0x8175ed0, event=0x80a03d8,
p=0x8175cf0) at nsGtkEventHandler.cpp:534
#55 0x408fd055 in gtk_marshal_BOOL__POINTER (object=0x8175ed0,
func=0x40083e20 <handle_motion_notify_event(_GtkWidget *, _GdkEventMotion *,
void *)>, func_data=0x8175cf0, args=0xbffff67c) at gtkmarshal.c:32
#56 0x408c1fa6 in gtk_handlers_run (handlers=0x8099da8, signal=0xbffff638,
object=0x8175ed0, params=0xbffff67c, after=0) at gtksignal.c:1909
#57 0x408c14b0 in gtk_signal_real_emit (object=0x8175ed0, signal_id=22,
params=0xbffff67c) at gtksignal.c:1469
#58 0x408bf7c0 in gtk_signal_emit (object=0x8175ed0, signal_id=22)
at gtksignal.c:552
#59 0x408f4710 in gtk_widget_event (widget=0x8175ed0, event=0x80a03d8)
at gtkwidget.c:2784
#60 0x408946a5 in gtk_propagate_event (widget=0x8175ed0, event=0x80a03d8)
at gtkmain.c:1295
#61 0x408939da in gtk_main_do_event (event=0x80a03d8) at gtkmain.c:752
#62 0x40939e86 in gdk_event_dispatch (source_data=0x0,
current_time=0xbffff9fc, user_data=0x0) at gdkevents.c:2086
#63 0x40965c83 in g_main_dispatch (current_time=0xbffff9fc) at gmain.c:647
#64 0x4096620f in g_main_iterate (block=1, dispatch=1) at gmain.c:854
#65 0x40966391 in g_main_run (loop=0x8084830) at gmain.c:912
#66 0x4089344b in gtk_main () at gtkmain.c:475
#67 0x4007c290 in nsAppShell::Run (this=0x8065310) at nsAppShell.cpp:152
#68 0x40014891 in nsAppShellService::Run (this=0x8064f20)
at nsAppShellService.cpp:154
#69 0x804a3bc in main (argc=1, argv=0xbffffb18) at nsAppRunner.cpp:334
Updated•26 years ago
|
Priority: P2 → P1
Comment 20•26 years ago
|
||
Also, sitting at www.mozilla.org is causing constant reflow,
joki suspects the status bar & a timer? probably the same bug.
Comment 21•26 years ago
|
||
The reflows are happening because the style system is being told be RDF content
that they are necessary. Specifically, all calls to
nsRDFElement::GetStyleHintForAttributeChange are coming back with the style hint
NS_STYLE_HINT_REFLOW. RDF content needs to be a bit more intelligent about when
to reflow and when to just redraw.
Updated•26 years ago
|
Component: Widget Set → Layout
Comment 22•26 years ago
|
||
Adding waterson. Chris, please check out vidur's last comment.
Comment 23•26 years ago
|
||
Added Hyatt; Dave -- see Vidur's comment. Was this the hack that we put in
because we had to force buttons to re-draw?
Assignee | ||
Comment 24•26 years ago
|
||
NS_STYLE_HINT_REFLOW is the correct hint to return, assuming that a button
could potentially change its size on a hover. If we return a more restrictive
hint, then we would be disallowing the toolbar button from growing.
The problem is that Gecko seems to be completely brain-dead regarding what it
should do once it's discovered that it needs to reflow. For example, if no
CSS rules are currently loaded that would cause the size of the button (or any
other elements to change when the attribute changes, then Gecko should be smart
enough to only apply a redraw to the affected element (rather than reflowing
everything).
This whole "GetStyleHintForAttributeChanged" stuff is utter nonsense. As
far as I can tell, it's a hack that dodges the real issue: the fact that any
attribute change could potentially cause a reflow or even a reframe, and it's up
to the style system to actually determine whether or not the reflow is
necessary.
Every time you use a hint on an attribute change that avoids a reflow, you're
creating a situation in which certain CSS2 rules (that are based off of that
attribute and that might grow/shrink an element, change the display of an
element, etc. etc.) just won't work.
What's broken about all of this is the design of this attribute hint system.
The hint should never be supplied. It should be computed. This should be fixed
and done properly so that we can truly be CSS2-compliant.
My 2 cents.
Comment 25•26 years ago
|
||
No. Reflow is NOT the right hint to return. Please refrain from critisizing
systems that you don't understand.
GetStyleHintForAttributeChange is only supposed to return hints for attribute
changes where it understands what the attribute change means. If it doesn't
understand, it can return NS_STYLE_HINT_UNKNOWN and the style system will
compute everything.
The content is NOT responsible for anticipating changes due to CSS style rules
with attribute selectors. The sytle system handles that (admitedly there is some
missing code, this system is still coming online). The content should only
reflect changes that the CONTENT will make to style as a result of a specific
attribute changing.
Content elements have an ability to map their attributes into style data. That
is what MapAttributesInto does. GetStyleHint is a corresponding method so that
the style system can get information about what specific attributes do when they
change. Nothing more, nothing less.
Since RDFElement has NO content style rule, it chooses NOT to map attributes
directly into style. So it should return NS_STYLE_HINT_CONTENT for ALL attribute
changes (since attributes DON'T represent style information). Getting style
rules with attribute selectors to apply correctly is my problem, not this bug.
Assignee | ||
Comment 26•26 years ago
|
||
I think I understand what's going on... but you've corrected me many times
before, so I could be wrong again... ;)
First of all, I returned reflow over unknown, because unknown was originally
causing a reframe, and the code for a reframe wasn't yet implemented... so I
ended up having to use reflow instead in order to dodge the NOT_YET_IMPLEMENTED
call.
I understand that I shouldn't be supplying a hint at all; that's what I
indicated in the rest of my post... that a hint shouldn't be necessary for XUL
attributes. If reframe is now implemented, then I can go back to specifying
UNKNOWN instead of REFLOW.
I believe my basic criticism still stands: the attribute hints, although useful
for optimizing what to do on certain attribute changes, result in rules in the
CSS that will not be obeyed.
Consider an attribute baz on a node foo that has a style hint of "render", thus
avoiding a reflow. A rule sequence like this won't work:
foo { display: list-item; list-style-image: etc. etc. }
foo[baz] { display: none; }
If a hint is specified for baz that prevents the reframe that must occur based
on the specified CSS rule, then the result is that the reframe won't occur.
Regardless of whether or not a hint is specified, there is still a potential for
a reflow or even a reframe, and I don't think that is taken into account when a
hint is specified.
Furthermore, on a reflow, when the size of elements don't change, the reflow
should be optimized so that only a redraw occurs. Even with a style hint of
reflow being specified on our buttons, the sizes of the buttons aren't changing,
and a reflow should take that into account.
So I claim that my two original criticisms of the system are still valid:
(1) Specifying a hint that is narrower than a reframe could result in CSS rules
that could trigger a reflow or reframe not being honored.
(2) On reflows where the sizes of elements don't change, the entire window
should not be invalidated. The element in question should just be redrawn.
Then again maybe I'm completely wrong. Give me a good stomping if I am.
Updated•26 years ago
|
Assignee: mcafee → rickg
Severity: normal → major
Comment 27•26 years ago
|
||
reassigning to rickg per some hallway conversations. Bumping severity to
major, since this blocks usable dogfood and could discourage external
developers from participating (unless they can fix it). I think this bug report
is pushing the capabilities of Bugzilla as a communication medium, perhaps a
meeting is in order to agree on an approach to resolving this?
Assignee | ||
Comment 28•26 years ago
|
||
Hmmm. I think much of the confusion on our part stems from the fact that
attribute selectors are still coming online. If I'm understanding you
correctly, you're saying that the attribute hint has nothing to do with whether
or not attribute selectors fire, and that it's up to you to get that working
properly regardless of what hint we specify.
Assuming that's true, then it is correct that we should be returning the CONTENT
style hint, since our attributes don't represent style changes. However, the
problem (which led me to use the REFLOW hint in the first place) is that we have
all sorts of XUL attribute selector rules that we're using everywhere, and we
need those rules to be applied when the attributes change. In order to avoid
writing an AttributeChanged method in every XUL frame that just does the same
thing (i.e., triggers a reflow), I went ahead and used the style hint REFLOW
instead.
If I change things over, all of these attribute selector rules will break unless
I explicitly reflow in my AttributeChanged methods. If I understand you
correctly, this would be the wrong thing to do anyway, since you're planning on
bringing code online that figures out what nodes match attribute selector rules
and making the necessary changes.
So what's the right thing for us to do for now? We're very dependent on
attribute selector rules at the moment, and they more or less work now, since
we're triggering reflows whenever attributes change.
Assignee | ||
Comment 29•26 years ago
|
||
BTW, I apologize for criticizing the attribute hint design. They did not mean
what I thought they mean. The confusion was because the code to handle
attribute selector rules on an attribute changed doesn't exist yet. My
apologies.
Comment 30•26 years ago
|
||
Yes. You understand it now (and welcome to the bleeding edge). Continue to use
the reflow hint for now. I'll have the rest of the attribute change code working
in a day or so (so that attribute selectors apply correctly to attribute
changes). When that's in, change your code to return a hint of CONTENT.
The fact that a local reflow causes the entire document to repaint is a seperate
issue which still needs to be addressed (by Troy I presume).
Comment 31•26 years ago
|
||
moving to m4
Comment 32•26 years ago
|
||
Is there a bug filed (perhaps against Troy) regarding
local-reflow/full-repaint? Is it going to get fixed for M4?
Comment 33•26 years ago
|
||
It has been a month now, and this problem is as bad as ever. Is any progress
being made on it?
Comment 34•26 years ago
|
||
the image rendering improvements i checked in last night helped this problem
alot. About 80% worth...
Assignee | ||
Comment 35•26 years ago
|
||
Actually, we managed to alleviate the problem by returning the CONTENT hint on
our nodes, and everything was working much better. Just recently (within the
last day or so), all of the nodes that had attributes changed (toolbar buttons,
tree rows, etc.) started reflowing again.
My theory is that Peter Linss landed his code that checks to see if any
attribute selector rules have now been matched (or unmatched), and his code is
probably triggering a reflow.
Can I suggest what I believe is a very simple optimization that would solve this
problem? What we have here is a situation where an object gets reflowed because
a CSS rule with an attribute selector is now matched (or no longer matched). It
seems like it's ok that the code triggers a reflow... what we need to do is make
reflow a little smarter.
When you reflow the object that was affected, and if its new size is exactly the
same as its old size, then the only thing that needs to be invalidated is the
object itself. If we can pull off this optimization (which doesn't sound very
hard to do), then we'll be golden.
Peter, am I right that your code is probably triggering reflows now, since we
have CSS rules that contain attribute selectors that are now being noticed by
this new code you checked in?
Comment 36•26 years ago
|
||
Well, maybe, but i can still cause linux to get waaaay behind in the rollover
feedback by moving the mouse quickly. It's easy to make it get on the order of
several seconds behind very quickly.
Comment 37•26 years ago
|
||
Well, maybe, but i can still cause linux to get waaaay behind in the rollover
feedback by moving the mouse quickly. It's easy to make it get on the order of
several seconds behind very quickly.
Comment 38•26 years ago
|
||
Yeah, I noticed the redraws got much better last week and
recently got worse again.
Reporter | ||
Comment 39•26 years ago
|
||
It's much worse this morning (3/26) than it was yesterday morning. But even
yesterday's build is worse than what I remember from earlier in the week and
late last week, when the buttons were almost keeping up with the mouse.
Comment 40•26 years ago
|
||
It doesn't look any better in this morning's optimized bits.
Comment 41•26 years ago
|
||
Yes, my code is in place to trigger reflows as needed by attribute selectors.
However, there are still too many attribute change notifications happening.
For example, mouse down sets active, then sets focus, where it could do
both at once.
Besides all that, none of the dynamic properties really should be reflected in
attributes anyway. This was a work-around until pseudo-classes were online
(they are now modulo support from the event state manager). You need to
coordinate with joki (and me, in email) to transition over to setting active,
hover and focus state in the event manager. Then remove the attribute setting
code, and change your style rules to use pseudo classes.
The style system will shortly be able to handle pseudo class changes
significantly faster than attribute changes (because I can detect the impact
faster).
There are still optimization opportunities in the reflow code that need to be
explored too.
Comment 42•26 years ago
|
||
This should be much better now due to improvements in style and UI code.
David -- please confirm.
Updated•26 years ago
|
Comment 43•26 years ago
|
||
Turn off double-buffering and go to a busy page, like the
Netscape home page, and then mouse-over the back/forward
buttons, I am counting what seems to be 3 full redraws. (Linux, current build)
Comment 44•26 years ago
|
||
*** Bug 4352 has been marked as a duplicate of this bug. ***
Comment 45•26 years ago
|
||
*** Bug 2575 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 46•26 years ago
|
||
Just in case anyone is keeping score, this got an order of magnitude worse on
3/30 vs. 3/29. The 3/30 build is virtually unusable.
Comment 47•26 years ago
|
||
Please compare navigator.xul (or whatever) in viewer and in apprunner. In
viewer, none of the JS handlers will run (because there is no appcore). In
apprunner, all the JS handlers will run. On win32, there is a noticable
difference: it means too much callback code is running on rollover. evaughan
checked in changes to navigator.xul so that it now uses box layout, and reflows
amazingly fast in viewer.
Comment 48•26 years ago
|
||
anyone know whats going on with this one?
Comment 49•26 years ago
|
||
*** Bug 4333 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•26 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M4 → M5
Assignee | ||
Comment 50•26 years ago
|
||
Moving this to M5. This is really sort of a silly bug at this point, because it
can really just be summed up as "Apprunner performance sucks." There isn't any
one reason for this. There are many reasons.
Comment 51•26 years ago
|
||
Fine, but we can't keep puttin this off; if we can't get decent performance then
we need to find out sooner rather than later. Since Chofmann wants one of the
foci of M5 to be performance, let's nail this for M5.
Comment 52•26 years ago
|
||
Fine, but we can't keep puttin this off; if we can't get decent performance then
we need to find out sooner rather than later. Since Chofmann wants one of the
foci of M5 to be performance, let's nail this for M5.
Comment 53•26 years ago
|
||
We should split this bug up into specific bugs,
I agree this bug isn't really doing us any good and
is a poor place-holder for "performance problems".
Comment 54•26 years ago
|
||
Split it up into what? The bug as originally reported is quite specific already.
However, we should start measuring performance, and report bugs where the
measurement or the perception is sub-par.
Comment 55•26 years ago
|
||
I was commenting per hyatt's comment, above.
Assignee | ||
Updated•25 years ago
|
Assignee: hyatt → rickg
Status: ASSIGNED → NEW
Assignee | ||
Comment 56•25 years ago
|
||
I am handing this bug off to rickg. There are two problems that need to be
addressed, and rickg can split this into two bugs and assign to the appropriate
Gecko engineers.
(1) Incremental reflow causes a repaint of the entire window.
(2) The computed style impact on an AttributeChanged is sometimes too severe
(e.g., the style impact assessment is sometimes reflow when it should only be
render).
Comment 57•25 years ago
|
||
Is that M5 milestone setting meaningful, or should we set it to M6 or M7 and
stop kidding ourselves?
Comment 58•25 years ago
|
||
Ok, I forked off two bugs for this bug, per hyatt's suggestion:
(1) http://bugzilla.mozilla.org/show_bug.cgi?id=5399
Incremental reflow causes a repaint of the entire window
(2) http://bugzilla.mozilla.org/show_bug.cgi?id=5400
The computed Style impact on an AttributeChanged is sometimes too severe
Anything else need to get forked off? Can we close this blanket bug now?
Comment 59•25 years ago
|
||
Tom -- this is critical fix. Please make it a priority. Peter may be some help.
Kipp knows about the reflow/paint problem.
Comment 60•25 years ago
|
||
Tom, I believe that the remaining issue here that isn't covered by other bugs,
is that the XUL code is still using the "pseudoclass" attribtue (& attribute
selectors) to reflect content state. The XUL code needs to change to use the
proper event state manager APIs instead (and we need to make sure that the
event state manager addresses their needs).
There is also GFX widget code that is using attributes to reflect content state
that needs to be updated as well.
Comment 61•25 years ago
|
||
Peter, some (most?) of us in xptoolkit land are ignorant about the event state
manager api's. Are they new? When did they go in?
Can you point us at some documentation or write up something quickly? We want our
code to do the right thing but Gecko keeps evolving out from under us (not a bad
problem to have, if you ask me ;)
Updated•25 years ago
|
Target Milestone: M5 → M6
Comment 62•25 years ago
|
||
Moving off M5 radar since Tom is out of town.
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 63•25 years ago
|
||
Guys, I don't know too much about coding, but I have been trying to use the
Mozilla milestone releases now since M3, and right up through M5, this seems to
have been a problem. And yes, it makes it painful for outsiders to try to help,
IMHO (it does me anyway). It makes it darned near impossible for me to test the
thing properly, since it is so hard for me to even use the Back/Forward/Reload
buttons. Anyway, I am wondering if bug #4818 is not related to this somehow??
Anyone??
Comment 64•25 years ago
|
||
I have, I think, the last event side piece of this fix to go in. The buttons
have moved to using the pseudoclasses for state change. In addition, I have
one last optimization that should get us down to a single repaint per
mouseover. After that I believe it's all Linux painting(/reflow?) issues.
Updated•25 years ago
|
Assignee: joki → ramiro
Status: ASSIGNED → NEW
Target Milestone: M6 → M7
Comment 65•25 years ago
|
||
Alright, small fix to reduce repaints/reflows checked in. I'm not sure who
owns this now but I'm pretty sure its not me. Going to pass to the unfortunate
ramiro, default holder of all Linux bugs. Also incrementing milestone.
Comment 66•25 years ago
|
||
Putting on [Perf] radar
Comment 67•25 years ago
|
||
*** Bug 7665 has been marked as a duplicate of this bug. ***
Updated•25 years ago
|
Updated•25 years ago
|
Assignee: ramiro → mcafee
Comment 68•25 years ago
|
||
This bug has recently been fixed. Im told that maybe some optimizations that
hyatt made took care of it.
Mousing over buttons in the apprunner toolbar no longer causes reflow and/or
repaint.
Im hesitant to mark it fixed cause you can still see the same effect when you
click anywhere on the content area - including xul buttons.
mcafee: im reassigning this to you. can you talk it over with hyatt and
determine whether you guys will add more tweaks as the above.
In the mean time, im working on other performance problems to alleviate this.
thanks
Comment 69•25 years ago
|
||
Adding hyatt, Dave please read ramiro's last comment.
Updated•25 years ago
|
Assignee: mcafee → hyatt
Comment 70•25 years ago
|
||
Reassigning to hyatt so he can take a look.
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M7 → M8
Assignee | ||
Comment 71•25 years ago
|
||
I don't know anything about this. I think joki was fixing stuff up. Moving off
my M7 radar.
Summary: [PP BLOCK] toolbar buttons eat CPU on mouseover → [PP] BLOCK toolbar buttons eat CPU on mouseover
Assignee | ||
Updated•25 years ago
|
Target Milestone: M8 → M15
Comment 72•25 years ago
|
||
Mouseover seems to work fine with the latest builds 1999080408.
Comment 73•25 years ago
|
||
I agree that it seems to work fine for the toolbars now in the Xlib port--that
seems as fast as Netscape 4.61 now. (The GTK+ port is also getting *much*
better, but just the same, still slower than it ought to be, IMHO).
However, while it works fine for the toolbars, it still seems dog slow for
Hyatt's XP menus.
Comment 74•25 years ago
|
||
in 19990831 mousing over toolbarbuttons still reflows entire window
(at least scrollbars flickers). This happens all buttons but not
in editor buttons 'B', 'I' and 'U'. What is different with those
compared to other titledbuttons?
Also notised that mousing over menubar reflows everything at firs time
(scrollbars flickers) but not when mousing over second time.
This is on linux/gtk
Reporter | ||
Comment 75•25 years ago
|
||
The "B", "I" and "U" buttons are different in that they're being rendered as
characters, not as bitmaps. Perhaps the current repaints are coming from
drawing images?
Comment 76•25 years ago
|
||
Toolbar mouseover is bug #11290.
Comment 77•25 years ago
|
||
I tracked this down to a call to SetFrameRate() which creates a timer that
Invalidates() everything again.
Comment 78•25 years ago
|
||
O k, now it looks really nice! No reflow/repaint from mousing over toolbar.
Ui is way faster now.? Something has changed ower weekend that fixed this.
Still mousing over links causes some replain and scrollbars flickers, but that
is maybe another bug?
Comment 79•25 years ago
|
||
Bug #11290 is marked fixed now.
Updated•25 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → WORKSFORME
Comment 80•25 years ago
|
||
resolving as worksforme, too general, there are other more specific bugs open.
Comment 81•24 years ago
|
||
This looks good on Mozilla build 2000091312 MN6. chrome mouse overs are very
responsive.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•