Re: NSSwitch drawRect bug
Hi all,
> Am 14.08.2023 um 08:55 schrieb Fred Kiefer :
>
>
>> Am 14.08.2023 um 08:36 schrieb H. Nikolaus Schaller :
>>
>>> Am 13.08.2023 um 22:40 schrieb dr_c...@me.com:
>>>
>>> Nice! Maybe even just drawSwitchInRect:dirtyRect:forState:enabled: Though,
>>> with your way, you could get more information from the control without
>>> having to enlarge method arguments every time we wanted to add something
>>> else.
>>>
>>> So maybe -(void) drawSwitch:(NSSwitch*)switch inRect:(CGRect)rect
>>> dirtyRect:(CGRect)dirtyRect {}
>>
>> I am not sure if passing the dirtyRect is necessary and useful.
>> The idea is that -drawRect: is usually called after setting a clipping rect
>> within -display so that drawing with bounds size is correct but will be
>> clipped away.
>
>
> This is not about correctness, as you wrote the NSView clipping will take
> care of that, this idea is about performance. When we pass on the dirty
> rectangle the drawing will be able to decide which parts actually require a
> redraw. This isn’t important for a small component like an NSSwitch but for
> something like NSMatrix or NSTableView it allows us to speed up drawing a
> lot. We only redraw and compute bits that will be visible.
> In general we should always draw a view within the assigned bounds and use
> the dirty rectangle to speed things up, where this is useful. Somebody should
> go through the NSView subclasses to see where corrections in the code are
> required.
Apple has a good description here:
https://developer.apple.com/documentation/appkit/nsview/1483686-drawrect
The dirty rect is a *hint* for *optimizing* drawing. It is not a bounds
dimension of a view which means that using it for scaling was also wrong. But
there may not only be a single dirty rect, there may be dirty regions.
Hence, the best solution would be if the drawing code for a View or Cell knows
about the view it is drawing into and can use methods like
-getRectsBeingDrawn:count: or -needsToDrawRect: This can easily be done easily
within -drawRect: but not within [GSTheme theme].
So passing the control view seems to be the right thing, until we find that
there is a +focusView in NSView. This should be the view which is currently
being drawn to.
Hence there is no need to pass down neither the dirtyRect nor a controlView to
a method like -drawSwitchInRect:forState:enabled: if it wants to optimize
drawing.
BR, hns
Re: NSSwitch drawRect bug
> Am 14.08.2023 um 08:36 schrieb H. Nikolaus Schaller :
>
>> Am 13.08.2023 um 22:40 schrieb dr_c...@me.com:
>>
>> Nice! Maybe even just drawSwitchInRect:dirtyRect:forState:enabled: Though,
>> with your way, you could get more information from the control without
>> having to enlarge method arguments every time we wanted to add something
>> else.
>>
>> So maybe -(void) drawSwitch:(NSSwitch*)switch inRect:(CGRect)rect
>> dirtyRect:(CGRect)dirtyRect {}
>
> I am not sure if passing the dirtyRect is necessary and useful.
> The idea is that -drawRect: is usually called after setting a clipping rect
> within -display so that drawing with bounds size is correct but will be
> clipped away.
This is not about correctness, as you wrote the NSView clipping will take care
of that, this idea is about performance. When we pass on the dirty rectangle
the drawing will be able to decide which parts actually require a redraw. This
isn’t important for a small component like an NSSwitch but for something like
NSMatrix or NSTableView it allows us to speed up drawing a lot. We only redraw
and compute bits that will be visible.
In general we should always draw a view within the assigned bounds and use the
dirty rectangle to speed things up, where this is useful. Somebody should go
through the NSView subclasses to see where corrections in the code are required.
Hope this clears things up,
Fred
Re: NSSwitch drawRect bug
> Am 13.08.2023 um 22:40 schrieb dr_c...@me.com:
>
> Nice! Maybe even just drawSwitchInRect:dirtyRect:forState:enabled: Though,
> with your way, you could get more information from the control without having
> to enlarge method arguments every time we wanted to add something else.
>
> So maybe -(void) drawSwitch:(NSSwitch*)switch inRect:(CGRect)rect
> dirtyRect:(CGRect)dirtyRect {}
I am not sure if passing the dirtyRect is necessary and useful.
The idea is that -drawRect: is usually called after setting a clipping rect
within -display so that drawing with bounds size is correct but will be clipped
away.
>
>> On Aug 13, 2023, at 1:10 PM, Fred Kiefer wrote:
>>
>> This is a common problem in our drawing code, especially in GSTheme. In the
>> better cases we hand on the view/controller to be drawn along with the
>> requested rectangle. That way the drawing code could still optimise the
>> process, by only drawing components that are actually visible.
>>
>> I will change the code as suggested by you.
>>
>> Cheers,
>> Fred
>>
>>> Am 11.08.2023 um 21:23 schrieb Austin Clow :
>>>
>>> In NSSwitch.m, the method for drawRect is as follows:
>>>
>>> - (void) drawRect: (NSRect)rect
>>> {
>>> [[GSTheme theme] drawSwitchInRect: rect
>>> forState: _state
>>>enabled: [self isEnabled]];
>>> }
>>>
>>> I believe it should be
>>>
>>> - (void) drawRect: (NSRect)rect
>>> {
>>> [[GSTheme theme] drawSwitchInRect: [self bounds]
>>> forState: _state
>>>enabled: [self isEnabled]];
>>> }
>>>
>>> As it is right now, when it redrawing a rect, it will redraw it within rect
>>> causing it to draw bigger and smaller depending not he redraw area. I'm not
>>> comfortable yet doing pull requests for a library I am largely unfamiliar
>>> with.
>>>
>>> I know the drawing method is kinda ugly right now. I am thinking about
>>> rewriting it.
>>
>
