[bug #40776] NSClipView should redraw when bounds change

[Jeff Teunissen]

URL:
<http://savannah.gnu.org/bugs/?40776>

Summary: NSClipView should redraw when bounds change
Project: GNUstep
Submitted by: Deek
Submitted on: Mon 02 Dec 2013 11:07:18 AM EST
Category: Gui/AppKit
Severity: 3 - Normal
Item Group: Bug
Status: None
Privacy: Public
Assigned to: None
Open/Closed: Open
Discussion Lock: Any

_______________________________________________________

Details:

When changing zoom factor, TextEdit just changes the clipview's bounds, on the
assumption that the clipview will automatically mark itself as needing
display. GNUstep's doesn't, so the display is messed up until the user
scrolls.

Patch attached, pulled out of my tree.




_______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Mon 02 Dec 2013 11:07:18 AM EST Name:
0001-NSClipView-redraw-when-bounds-size-changes.patch Size: 1kB By: Deek

<http://savannah.gnu.org/bugs/download.php?file_id=29754>

_______________________________________________________

Reply to this item at:

<http://savannah.gnu.org/bugs/?40776>

_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/

[Fred Kiefer]

Update of bug #40776 (project gnustep):

Status: None => Ready For Test
Assigned to: None => FredKiefer
Open/Closed: Open => In Test

_______________________________________________________

Follow-up Comment #1:

Thank you for the bug report and the patch. I committed a slightly modified
version of your patch. Please give it a try.

_______________________________________________________

Reply to this item at:

<http://savannah.gnu.org/bugs/?40776>

_______________________________________________
Nachricht gesendet von/durch Savannah
http://savannah.gnu.org/

[Jeff Teunissen]

Follow-up Comment #2, bug #40776 (project gnustep):

I don't think that will work out very well.

When doing the patch, the very first thing I tried was marking only the
document view as needing display, but the document view does not have to fill
the clipview.

Because of this fact, there may be quite a lot of area that /was/ covered by
the document view that no longer is.

If you have a 1000x1000 documentview and scale or otherwise resize it to
100x100, telling the documentview to redraw will only redraw the 100x100 area
the documentview now fills, leaving the rest of the clipview unchanged (even
though it's still full of the clipview's former contents).

That's why the clipview (_and_ the documentview inside it, but that's handled
anyway) needs to be redrawn.


_______________________________________________________

Reply to this item at:

<http://savannah.gnu.org/bugs/?40776>

_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/

[Fred Kiefer]

Update of bug #40776 (project gnustep):

Open/Closed: In Test => Open

_______________________________________________________

Follow-up Comment #3:

Could you please try the code and see if the problem you are expecting really
occures?
I would like to have the code in setBounds: and setBoundsSize: in line with
the code in the method setBoundsOrigin: and there we only set the
_documentView as needing redisplay. This may be wrong, but we never had a bug
report on this and that code has been in GNUstep for a long time. For this
reason I would only like to change this, if there is a proven problem with
it.

I moved this bug report back to open to make it easier to find for others
interested.

_______________________________________________________

Reply to this item at:

<http://savannah.gnu.org/bugs/?40776>

_______________________________________________
Nachricht gesendet von/durch Savannah
http://savannah.gnu.org/

[Jeff Teunissen]

Follow-up Comment #4, bug #40776 (project gnustep):

Yes, I'm sure. In the attached screenshot, I set the zoom factor to 200%
(which redrew to fill the window, as it should), then set it back to zero,
which then resulted in a redrawing of the document view...leaving the portions
of the clip view not covered by the document view unchanged.

If the new bounds are not a superset of the old bounds, then the clipview
itself must be redrawn. Only if the new bounds are strictly greater than the
old ones can you get away with not redrawing the clipview itself.


(file #29804)
_______________________________________________________

Additional Item Attachment:

File name: ClipView-drawing.png Size:170 KB


_______________________________________________________

Reply to this item at:

<http://savannah.gnu.org/bugs/?40776>

_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/

[Fred Kiefer]

Update of bug #40776 (project gnustep):

Open/Closed: Open => In Test

_______________________________________________________

Follow-up Comment #5:

You screen shot shows the problem quite clearly.

Your original proposal is still wrong though, there you use the
documentVisibleRect to invalidate self, which is missing two different
coordinate systems. Converting documentVisibleRect back to self gives _bounds,
which is the same as calling [self setNeedsDisplay: YES]. And I prefer to use
that instead.

I am not happy with that change, but in the end we might end up with adding
that call in NSView setBounds: anyway. Why not start doing it here.

_______________________________________________________

Reply to this item at:

<http://savannah.gnu.org/bugs/?40776>

_______________________________________________
Nachricht gesendet von/durch Savannah
http://savannah.gnu.org/

[Jeff Teunissen]

Follow-up Comment #6, bug #40776 (project gnustep):

Indeed, -documentVisibleRect is in the document view's coordinate system, I
failed to notice that when I replaced [self setNeedsDisplay: YES] with it, and
it still /seemed to/ work correctly, so I left it alone.

If you want to only redraw the whole clip view when the document view shrinks,
then one could do

:
:
NSRect oldBounds = [self bounds];
[super setBounds: b];
if (NSContainsRect(b, oldBounds))
{
[_documentView setNeedsDisplayInRect: [self documentVisibleRect]];
}
else
{
[self setNeedsDisplay: YES];
}
:
:

but that may be overkill, and just easier to always redraw when the bounds
change.


_______________________________________________________

Reply to this item at:

<http://savannah.gnu.org/bugs/?40776>

_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/

[Jeff Teunissen]

Follow-up Comment #7, bug #40776 (project gnustep):

Or, perhaps, using NSUnionRect(b, oldBounds) and converting to figure out the
actual dirty rect.


_______________________________________________________

Reply to this item at:

<http://savannah.gnu.org/bugs/?40776>

_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/

[Fred Kiefer]

Update of bug #40776 (project gnustep):

Status: Ready For Test => Fixed
Open/Closed: In Test => Closed


_______________________________________________________

Reply to this item at:

<http://savannah.gnu.org/bugs/?40776>

_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/