Closed Bug 213312 Opened 18 years ago Closed 18 years ago

Search field redraws too often


(Camino Graveyard :: Toolbars & Menus, defect)

Not set


(Not tracked)



(Reporter: pgauriar, Assigned: mikepinkerton)



(Keywords: helpwanted)


(1 file)

The toolbar search field in nightlies with the toolbar search field (20030410) redraws too often, 
thus causing performance issues when scrolling (and possibly general rendering speed issues).

To view this behavior:

1) Launch Camino.  Make sure the toolbar search field is in the toolbar.
2) Launch Quartz Debug (/Developer/Applications/Quartz
3) Turn on the "Flash screen updates" checkbox in Quartz Debug.
4) Type in the location bar or scroll in Camino

Actual results: The toolbar search field flashes yellow.
Expected results: The toolbar search field should only flash yellow when it is changing
Confirmed using 20030715
Attached patch Temp fixSplinter Review
This fixes it, but it causes another problem where the focus ring draws
incorrectly if the field is the first responder when the window is switched to
(the ends are drawn wrong).  It corrects itself after the first cursor blink,
so this might not be considered as big of a deal as the performance issue

Please test, I don't have a Jaguar system to test on right now and have only
tested in my own test app on Panther.
Could someone test this please, this will improve Camino's speed.

Prachi is this a patch file? If so how do I patch my version of Camino or do we
really have to make a new build?
*** Bug 213277 has been marked as a duplicate of this bug. ***
I need some help figuring out how to get the field's focus ring to draw correctly.

Keywords: helpwanted
David Haas's unofficial build of Camino has this patch applied so I was able to
test on 1.2.8.

First impression is that it gives Camino a nice little spead improvement, thanx.
Now there are also some bugs in this patch:

- while typing, the search field doesn't update on every key down. The result is
that when entering more characters than the width of the field allows characters
start being written over each other.
- other issues: 1) the search title (Google) isn't dimmed if the search field is
inactive, 2) when selecting another title (Google Images) the search field
doesn't display the selection when it's inactive.
- the focus ring has some painting probelms on the curved sides, but this is a
minor bug people won't even notice.
I don't have time to work on this anymore.  Giving it up so that I'm not the bottleneck.
Assignee: pgauria → pinkerton
Target Milestone: --- → Camino0.8
Let's not forget this. I recall this patch having a nice speed gain when
scrolling and such it just had some visula issues.
Somebody with a 10.2.x box should make a test app on 10.3.x that includes a
standard Apple search widget. I wouldn't be surprised if it existed in 10.2.x
and there just weren't API docs or an IB interface for it. If that is the case
we can use it when we dump support for 10.1.x...
there is a rounded search widget on 10.2 (you can even get to it from IB) but it
still doesn't do all the stuff we need for the popup menu. there's gotta be a
way to fix this that doesn't require dropping 10.1 support. We've gone this far,
let's finish the job.

Do we understand the root cause of why it flashes? What's triggering the
invalidations? That's the most important question in my mind.

(for my own notes, this does still happen on panther. I was under the impression
it did not).
The patch Prachi made worked fine aprat from the fact that it didn't update
correctly opposed to always updating.

Comment #6 describes the issues with the patch (you feel the speed gane).
> Do we understand the root cause of why it flashes? What's triggering the
> invalidations? That's the most important question in my mind.

While I haven't looked at this in awhile, commenting out line 215 in SearchTextFieldCell.m removed 
the extra redraw issues.  Other changes just tweak drawing of the focus ring. Line 214-215 (for 

214   // Invalidate the focus ring
215   [controlView setKeyboardFocusRingNeedsDisplayInRect:cellFrame];

The Apple's documentation says that this method just invalidates a region large enough to include 
the focus ring.

In the past five minutes looking at this, I have an ideas that _might_ make this better, but it's really 
a shot in the dark.

227   // Draw the text field
228   [self setShowsFirstResponder:NO];
229   [super drawWithFrame:[self textFieldRectFromRect:cellFrame] inView:controlView];
230   [self setShowsFirstResponder:wasShowingFirstResponder];

For this code snippet, change line 229 to 

[super drawInteriorWithFrame:[self textFieldRectFromRect:cellFrame inView:controlView];

I'm not sure how much that will help, but it seems like only the text needs to be drawn at that 
point, so redrawing everything doesn't seem to make a lot of sense.  Try this both with and without 
the patch. 
the best i could get it was for it to repaint entirely *only* when it has focus
and is in the key window. all other times (the norm) it no longer repaints
itself.  When we drop 10.1 support, we can move to the real search field which
won't have this issue.
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.