Investigate NSCell drawing quirks

VERIFIED FIXED

Status

()

Core
Widget: Cocoa
VERIFIED FIXED
10 years ago
5 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

Trunk
All
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

(Assignee)

Description

10 years ago
Right now we're using NSCells to draw pushbuttons, checkboxes and radiobuttons. However, our current setup triggers some odd bugs; e.g. things are drawn upside down on some platforms and not on others (see also bug 464353) which we work around using a flip transform. Moreover, when trying to draw NSSearchFieldCells (bug 450800) or NSPopUpButtonCells (bug 399030), we get really weird drawing bugs on 10.4.

I want to use this bug to find the answers to the following questions:
 - What triggers the mirror bugs?
 - What are the rendering differences between 10.4, 10.5 and 10.6?
 - What does -[NSCell drawWithFrame:inView:] need the NSView for?
 - Why do we use [NSView focusView] in that call?
 - What's up with [NSView focusView] being nil? (see bug 444864 comment 24)

And ultimately,
 - Is there a way of quirk-free drawing that doesn't need platform-specific
   setups?
(Assignee)

Comment 1

10 years ago
Created attachment 348323 [details]
Test app as an XCode project [zip]

I made this test app on 10.4; it covers all the important ways to draw NSCells.
(Assignee)

Comment 2

10 years ago
Created attachment 348324 [details]
Test app executable [zipped .app]
(Assignee)

Comment 3

10 years ago
Created attachment 348325 [details]
10.4 screenshot

This is awesome.
(Assignee)

Comment 4

10 years ago
Created attachment 348326 [details]
the code
(Assignee)

Comment 5

10 years ago
Marcia, could you make a screenshot of attachment 348324 [details] on 10.6?
(Assignee)

Comment 6

10 years ago
Created attachment 348442 [details]
10.5 screenshot
(Assignee)

Comment 7

10 years ago
This is what I found out on 10.4.

(In reply to comment #0)
>  - What does -[NSCell drawWithFrame:inView:] need the NSView for?

Among other things, it asks for [view isFlipped]. If isFlipped returns NO here, we get the drawing bugs that are shown in column 2 / 3.

You can test it in the test app using flippedOnDraw: {YES or NO} and inFocusView:NO.

>  - Why do we use [NSView focusView] in that call?

I think it's a leftover from bug 418497.
Before that bug, we used NSImages as buffers. When drawing into an NSImage, you surround your drawing with calls [image lockFocus] and [image unlockFocus]. [image lockFocus] creates an offscreen NSView and sets this as [NSView focusView].
(Now [[NSView focusView] isFlipped] returns what you passed to to [image setFlipped:].)

>  - What's up with [NSView focusView] being nil? (see bug 444864 comment 24)

I found out that (at least on 10.4) [NSView focusView] becomes nil as soon as +[NSGraphicsContext setCurrentContext:...] is called.
For our current code this means: Even if [NSView focusView] isn't nil at the beginning of DrawCellWithScaling, it will always be nil when we draw the cell, because we always call setCurrentContext before we draw.
So we're effectively passing nil every single time.

>  - What triggers the mirror bugs?

The mirror bugs occur when [view isFlipped] returns NO.
[nil isFlipped] evaluates to NO.
(Assignee)

Comment 8

10 years ago
Created attachment 349012 [details]
10.6 screenshot
(Assignee)

Comment 9

10 years ago
The patch in bug 450800 uses the drawing strategy shown in column 5 (without buffer) and column 6 (with buffer) of the screenshots. This seems to work reliably on all platforms, so the answer to the last question in comment 0 is apparently "Yes". :-)
(Assignee)

Comment 10

10 years ago
(In reply to comment #7)
> (In reply to comment #0)
> >  - What does -[NSCell drawWithFrame:inView:] need the NSView for?
> 
> Among other things, it asks for [view isFlipped]. If isFlipped returns NO here,
> we get the drawing bugs that are shown in column 2 / 3.

Actually it's the bug shown in column 7. Column 2 and 3 additionally use a flip transform.

> I think it's a leftover from bug 418497.

I.e. it should have been removed in bug 418497 because that removed the use of NSImage. Before bug 418497, using [NSView focusView] made a lot of sense.
(In reply to comment #7)

>>  - What's up with [NSView focusView] being nil? (see bug 444864 comment 24)
>
> I found out that (at least on 10.4) [NSView focusView] becomes nil
> as soon as +[NSGraphicsContext setCurrentContext:...] is called.
> For our current code this means: Even if [NSView focusView] isn't
> nil at the beginning of DrawCellWithScaling, it will always be nil
> when we draw the cell, because we always call setCurrentContext
> before we draw.  So we're effectively passing nil every single time.
>
>>  - What triggers the mirror bugs?
>
> The mirror bugs occur when [view isFlipped] returns NO.  [nil
> isFlipped] evaluates to NO.

Very interesting!  I've dug further into this (as I'll explain below).
But I've more-or-less confirmed your comments.  Thanks, Markus!  This
is a real advance in our understanding of how [NSView focusView] works
and how [NSCell drawWithFrame:inView:] uses its inView: parameter.

I hacked out the following by looking at the output of class-dump
(http://www.codethecode.com/projects/class-dump/) from the AppKit
framework's binary, by fooling around in gdb (particularly in
nsNativeThemeCocoa.mm's DrawCellWithScaling() function), and by
passing a special object to [NSCell drawWithFrame:inView:] (as its
inView: parameter) that inherits from NSObject and only has isFlipped
and currentEditor methods.

Every NSGraphicsContext object (every concrete subclass of
NSGraphicsContext) has an NSFocusStack object associated with it.  As
you'd expect, each NSFocusStack object is an array of elements (class
_NSFocusStackElement), the top of which corresponds to the NSView
object that currently has the drawing focus (whose lockFocus method
has been called, usually as part of the preparation for calling its
drawRect: method).

DrawCellWithScaling() is usually (probably always) called with
[ChildView drawRect:] on the stack for the native object (a ChildView
object) corresponding to the nsIFrame into which Gecko is drawing some
kind of control (e.g. a button).  So we are between calls to lockFocus
and unlockFocus on this ChildView object.  So the current focus stack
(the one belonging to the current NSGraphicsContext) is non-empty.
And so [NSView focusView] returns non-nil (in fact it returns the
ChildView object).

But the CGContextRef ('cgContext') passed in to DrawCellWithScaling()
was created by a call to
nsChildView::GetNativeData(NS_NATIVE_PLUGIN_PORT_CG), almost certainly
made when no call to [NSView lockFocus] or [NSView drawRect:] was on
the stack.  So 'cgContext's focus stack is almost certainly always
empty.

And I've seen that the CGContextRef ('ctx') created by a call to
CGBitmapContextCreate() inside DrawCellWithScaling() also always has
an empty focus stack -- probably because it's never yet been made the
current context.

So when you use [NSGraphicsContext setCurrentContext:] to make either
'cgContext' or 'ctx' the current context, an empty focus stack is made
current, and [NSView focusView] will return nil.

By trial and error I found that I could pass an object to [NSCell
drawWithFrame:inView:] (as its inView: parameter) that isn't even an
NSView object (one that inherits from NSObject), without apparent ill
effect.  All it needs are an isFocused method that returns FALSE and a
currentEditor method that returns nil.  It appears that no other
methods (not inherited from the NSObject class) are ever called on it
(or I would have seen "invalid selector" errors).  So Markus may be
right that isFlipped (and currentEditor) are called on whatever's
passed to inView: even it it's nil.  Or maybe if it's nil,
drawWithFrame:inView: assumes default return values for isFlipped and
currentEditor (FALSE and nil, respectively).  In either case the
result is the same.

I also never saw [NSView focusView] called from [NSCell
drawWithFrame:inView:].  So apparently the code in that method doesn't
care what NSView object has the current graphics focus, or use Cocoa
to do any low-level drawing (I suppose it must use lower-level calls).
(Assignee)

Comment 12

10 years ago
Investigation completed, bug 450800 landed.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Depending bugs are fixed and verified. Looks like we can verify this bug too.
Status: RESOLVED → VERIFIED

Updated

5 years ago
See Also: → bug 863104
You need to log in before you can comment on or make changes to this bug.