Closed
Bug 516924
Opened 15 years ago
Closed 15 years ago
Cocoa unnecessarily repaints our views as soon as we paint a focus ring
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
status1.9.1 | --- | .6-fixed |
People
(Reporter: mstange, Assigned: mstange)
References
()
Details
(Keywords: regression)
Attachments
(1 file)
10.67 KB,
patch
|
jaas
:
review+
dveditz
:
approval1.9.1.6+
|
Details | Diff | Splinter Review |
When debugging the repaints in bug 424715, I noticed that drawRect is asked to repaint the whole view, even though we only call setNeedsDisplayInRect for much smaller rects. And not only that: the repainted rect was even larger than the view, expanded by 3 pixels to the top and to the bottom. Then I remembered a paragraph from http://developer.apple.com/mac/library/releasenotes/Cocoa/AppKitOlderNotes.html#X10_5Notes : > To help guarantee correct redraw of focus rings, AppKit may automatically draw > additional parts of a window beyond those that application code marked as > needing display. It may do this for the first responder view in the > application's key window, if that first responder view's focusRingType property > is set to a value other than NSFocusRingTypeNone. Any view that can become > first responder, but doesn't draw a focus ring, should have its focusRingType > set to NSFocusRingTypeNone to avoid unnecessary additional redraw. So I added [self setFocusRingType:NSFocusRingTypeNone] to ChildView's init method, but it didn't make any difference. Playing around some more, I discovered that the additional redraws only started after I had clicked the "Basic Render" button, i.e. only after we rendered a button with a focus ring. The rest was easy. When we draw a button with a focus ring, we first call setShowsFirstResponder:YES on the NSButtonCell, and then pass the NSView that contains our button frame to [button drawWithFrame:inView:]. This seems to mark our NSView permanently. We can avoid that by simply using a pseudo view instead of the real view - whether the used view actually contains our button doesn't matter, as the investigation in bug 465069 has shown. So that's what my patch does. I'm also adding setFocusRingType: NSFocusRingTypeNone (even though I couldn't see a difference) because Webkit does it, too.
Flags: blocking1.9.2?
Attachment #400976 -
Flags: review?(joshmoz)
Comment on attachment 400976 [details] [diff] [review] v1 Looks good but please file a bug with Apple about NSFocusRingTypeNone not stopping the invalidations and record the Apple bug number here.
Attachment #400976 -
Flags: review?(joshmoz) → review+
this is potential perf worth blocking on, blocking1.9.2+
Flags: blocking1.9.2? → blocking1.9.2+
Assignee | ||
Comment 4•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/697b2063d06f
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
Keywords: regression
Target Milestone: --- → mozilla1.9.3a1
Assignee | ||
Comment 5•15 years ago
|
||
Filed rdar://problem/7229728
Assignee | ||
Comment 6•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/25e1253030f4
status1.9.2:
--- → beta1-fixed
Assignee | ||
Comment 7•15 years ago
|
||
Comment on attachment 400976 [details] [diff] [review] v1 I think we can also take this on 1.9.1 - it's a regression from 1.9.0 and the fix is reasonably safe. I looked into writing a test for this, but I'm afraid doing so seems impossible without bug 510970.
Attachment #400976 -
Flags: approval1.9.1.4?
Updated•15 years ago
|
Attachment #400976 -
Flags: approval1.9.1.4? → approval1.9.1.5?
Comment 8•15 years ago
|
||
Comment on attachment 400976 [details] [diff] [review] v1 Approved for 1.9.1.5, a=dveditz for release-drivers
Attachment #400976 -
Flags: approval1.9.1.5? → approval1.9.1.5+
Assignee | ||
Comment 9•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4c4bdcb43345
status1.9.1:
--- → .5-fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•