Re: gui patch for review: -[NSView setBounds*:] - remove setNeedsDisplay: YES calls
I investigated this and committed a fix for scrolling in a zoomed TextEdit document yesterday because the fix was quite simple (r34614). On 2012-01-23, at 4:05 PM, Eric Wasylishen wrote: I just found a bug caused by re-enabling copy-on-scroll. Open TextEdit (https://github.com/ericwa/TextEdit), open a document or paste in some text, Choose Format-Wrap to Page, set the zoom to 150%, and scroll horizontally, slowly. You'll see horizontal blurring. Seems that the regions being copied aren't pixel-aligned. Should we just disable copy-on-scroll for this release? Ship with this bug? Try using -centerScanRect in -[NSView scrollRect: (NSRect)aRect by: (NSSize)delta]? I just fear trying to fix it now will create more bugs at the last minute... Eric On 2012-01-20, at 11:54 PM, Eric Wasylishen wrote: Hi, This patch reverts part of r32955 which I committed last april but I now see was a mistake, and I just discovered is breaking the copy-on-scroll behaviour of NSClipView - we end up always redrawing the entire visible portion of the document view right now. In r32955 I added [self setNeedsDisplay: YES] calls to -[NSView setBounds:], -setBoundsOrigin:, and -setBoundsSize:, even though these are documented explicitly as not marking the view for needing display. (With r32955 I was trying to fix a bug in TextEdit, which calls setBoundsSize: on NSClipView when you change the page zoom, and expects the view to mark itself as needing display. This is actually a bug in TextEdit - it should mark the clip view as needing redisplay itself.) -Eric NSViewBoundsRemoveSetNeedsDisplay.diff ___ Gnustep-dev mailing list Gnustep-dev@gnu.org https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: gui patch for review: -[NSView setBounds*:] - remove setNeedsDisplay: YES calls
Ugh, r34614 introduced more bugs itself (resizing the panes in Grr can make the table headers change size). I should know this would happen if I tried to change things during a code freeze... Fred, do you think we should just ship the release with copy-on-scroll disabled? That would fix the flickering in FisicaLab reported by German, and the blurring I reported here in zoomed TextEdit. We could turn it back on after the release and try to fix those bugs. Eric On 2012-01-24, at 11:05 AM, Eric Wasylishen wrote: I investigated this and committed a fix for scrolling in a zoomed TextEdit document yesterday because the fix was quite simple (r34614). On 2012-01-23, at 4:05 PM, Eric Wasylishen wrote: I just found a bug caused by re-enabling copy-on-scroll. Open TextEdit (https://github.com/ericwa/TextEdit), open a document or paste in some text, Choose Format-Wrap to Page, set the zoom to 150%, and scroll horizontally, slowly. You'll see horizontal blurring. Seems that the regions being copied aren't pixel-aligned. Should we just disable copy-on-scroll for this release? Ship with this bug? Try using -centerScanRect in -[NSView scrollRect: (NSRect)aRect by: (NSSize)delta]? I just fear trying to fix it now will create more bugs at the last minute... Eric On 2012-01-20, at 11:54 PM, Eric Wasylishen wrote: Hi, This patch reverts part of r32955 which I committed last april but I now see was a mistake, and I just discovered is breaking the copy-on-scroll behaviour of NSClipView - we end up always redrawing the entire visible portion of the document view right now. In r32955 I added [self setNeedsDisplay: YES] calls to -[NSView setBounds:], -setBoundsOrigin:, and -setBoundsSize:, even though these are documented explicitly as not marking the view for needing display. (With r32955 I was trying to fix a bug in TextEdit, which calls setBoundsSize: on NSClipView when you change the page zoom, and expects the view to mark itself as needing display. This is actually a bug in TextEdit - it should mark the clip view as needing redisplay itself.) -Eric NSViewBoundsRemoveSetNeedsDisplay.diff ___ Gnustep-dev mailing list Gnustep-dev@gnu.org https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: gui patch for review: -[NSView setBounds*:] - remove setNeedsDisplay: YES calls
On 24.01.2012 20:01, Eric Wasylishen wrote: Ugh, r34614 introduced more bugs itself (resizing the panes in Grr can make the table headers change size). I should know this would happen if I tried to change things during a code freeze... This is a strange problem to be caused by that change. I can reproduce this problem with Grr and when reverting your change it is definitely gone. Still I don't quite see how the two are related. I did some debugging here and it seems that the problem starts when we get values below zero. From that I would conclude that the issue is rather that due to rounding errors in the transformations we return values that are not suitable for the scroll view. We could try to avoid this by doing the transformation to pixel space and back again before doing the clipping. That is move this code up before the load of if statements in this method. This of course will only work when the bounds of the clip view are pixel aligned. We need to ensure that in some other way. With that change in place Grr behaves again as before. Of course there may be other applications that get affected. Fred, do you think we should just ship the release with copy-on-scroll disabled? That would fix the flickering in FisicaLab reported by German, and the blurring I reported here in zoomed TextEdit. We could turn it back on after the release and try to fix those bugs. I replied to that question already in my last mail. And I am really not sure what the best way forward is here. ___ Gnustep-dev mailing list Gnustep-dev@gnu.org https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: gui patch for review: -[NSView setBounds*:] - remove setNeedsDisplay: YES calls
I just found a bug caused by re-enabling copy-on-scroll. Open TextEdit (https://github.com/ericwa/TextEdit), open a document or paste in some text, Choose Format-Wrap to Page, set the zoom to 150%, and scroll horizontally, slowly. You'll see horizontal blurring. Seems that the regions being copied aren't pixel-aligned. Should we just disable copy-on-scroll for this release? Ship with this bug? Try using -centerScanRect in -[NSView scrollRect: (NSRect)aRect by: (NSSize)delta]? I just fear trying to fix it now will create more bugs at the last minute... Eric On 2012-01-20, at 11:54 PM, Eric Wasylishen wrote: Hi, This patch reverts part of r32955 which I committed last april but I now see was a mistake, and I just discovered is breaking the copy-on-scroll behaviour of NSClipView - we end up always redrawing the entire visible portion of the document view right now. In r32955 I added [self setNeedsDisplay: YES] calls to -[NSView setBounds:], -setBoundsOrigin:, and -setBoundsSize:, even though these are documented explicitly as not marking the view for needing display. (With r32955 I was trying to fix a bug in TextEdit, which calls setBoundsSize: on NSClipView when you change the page zoom, and expects the view to mark itself as needing display. This is actually a bug in TextEdit - it should mark the clip view as needing redisplay itself.) -Eric NSViewBoundsRemoveSetNeedsDisplay.diff ___ Gnustep-dev mailing list Gnustep-dev@gnu.org https://lists.gnu.org/mailman/listinfo/gnustep-dev
gui patch for review: -[NSView setBounds*:] - remove setNeedsDisplay: YES calls
Hi,This patch reverts part of r32955which I committed last april but I now see was a mistake, andI just discovered is breaking the copy-on-scroll behaviour of NSClipView - we end up always redrawing the entire visible portion of the document view right now.Inr32955I added [self setNeedsDisplay: YES] calls to -[NSView setBounds:], -setBoundsOrigin:, and -setBoundsSize:, even though these are documented explicitly as not marking the view for needing display.(Withr32955Iwas trying to fix a bug in TextEdit, which calls setBoundsSize: on NSClipView when you change the page zoom, and expects the view to mark itself as needing display. This is actually a bug in TextEdit - it should mark the clip view as needing redisplay itself.)-Eric NSViewBoundsRemoveSetNeedsDisplay.diff Description: Binary data ___ Gnustep-dev mailing list Gnustep-dev@gnu.org https://lists.gnu.org/mailman/listinfo/gnustep-dev
