My understanding of your change is that the "for (g = x->gl_list; g; g = g->g_next)" loop should just be eliminated, since the code you added would basically mean that within "if (!nogoprect && !x->gl_goprect)" x->gl_goprect would always be set to 1.

Indeed, that does appear to be an even simpler fix.

I tried a bunch of tests but I couldn't see any difference besides with the red rectangle appearing when no objects are present on the GOP canvas. Do you have any example patches and/or bug reports that you are referring to?

Not sure if this affects other platforms but on Linux you can try the following *without the patch applied*:

1) create [pd something] with no objects inside it 2) enable GOP *and* hide text options on [pd something] 3) hit OK and save patch 4) close patch 5) reopen patch and hide text option will be disabled (or more accurately never saved

You can also inspect the patch and you will discover that readwrite is saving GOP settings using the old pre-0.38 format which also does not help (meaning it cannot save hide text option).

Another test that is a bit more complicated is creating cascading GOP patchers with visible objects inside them. When I did not have the aforesaid patch applied, I would get weird results when disabling GOP while having objects embedded within it. This would sometimes leave stale objects on the screen and eventually lead to crashes.

With this thing patched, I've yet to experience those again.

For future reference, please post patches with only the necessary changes, and do not include any formatting changes. This makes it much easier to read the patches. I attached a reformatted version of your patch for an example.

Indeed, refactored diff is right, except I would also appreciate listing my name next to the edited part to somehow validate 4 hours I spent dealing with this silly but important bug :-)

Please see attached.

Ico

On Tue, 2010-05-18 at 13:30 -0400, Ivica Ico Bukvic wrote:

All right, the following fixes scrolling on Linux once and for all. It should be also wrapped in a way that does not disturb Win and OSX (even though they are in a need of the same fix).

Please note the following applies to: Linux/OSX/Windows Pd 0.42.5 vanilla and extended Tcl/Tk 8.4 & 8.5

Basically, text is incorrectly reported by bbox all call so it screws scrollbars up. E.g. try the attached problem_1.pd patch and you'll notice there are vertical scrollbars even though there shouldn't be any. This makes tight GUI crafting a real pain. Also, please note that on smaller font sizes the problem is less apparent but nonetheless still there.

Solution: calculating text size separately from other objects to obtain total box size. This, however, has to be done *per font* due to tcl/tk's buggy implementation.

Attached patch also reinvokes getscrollbar after cut/paste/undo/redo (undo/redo is patched in g_editor.c as otherwise it occurs out of sync before undo/redo has been processed in g_editor.c, resulting in a erroneous scroll position).

The current scrolling algorithm should to the best of my knowledge pass all the existing test Hans has listed on his gui-rewrite page with one notable difference:

This approach assumes that the canvas should always fit objects the best it can to avoid using scrollbars (unlike traditional approach that has an invisible center around 0 0). This does sometimes result in patch shifting a bit off-center (but never outside the view unless the entire patch is too large to fit the saved window size)--this is due to the fact that currently Pd has no way of telling from the saved file where the viewport should be. This can be easily circumvented using the cnv object which still yields IMHO better results than the traditional system.

Cheers!

Ico

Oops, forgot to include the right pd.tk patch. This one includes proper realignment of the pop-up window (right-click), plus something I found rather handy, the windows that pop-up always appear next to the mouse cursor, rather than a random place on-screen.

So, attached are all the files, plus the correct pd.tk patch. Hence, please disregard patches I sent out in my previous email.

Best wishes,

Ico

All,

As I put finishing touches on the improved scrolling algorithm, the last object I just added to dynamic resizing of the scrollbars is the "graph" object. After messing with it I discovered that it was very unstable and am wondering what is its use given that its entire functionality is essentially replicated in the GOP/array. Hence, I am wondering is this object for all intents and purposes deprecated or am I missing something?

ico

I briefly checked this out and it looks like a substantial effort that we should get merged in. But I think its too large to merge into Pd- extended 0.42.5 at this stage of the release cycle. I believe I mentioned before that working with this GUI code was a dead end since 0.43 includes the new GUI code.

This stuff should be adapted to the code in pd-gui-rewrite/0.43 branch. If it gets included there, it'll then make it into Pd vanilla and Pd-extended.

.hc

On May 19, 2010, at 3:04 PM, Ivica Ico Bukvic wrote:

OK, here's a complete patch that fixes most (all?) issues with scrolling algorithm listed below.

Please note the following applies to: Linux/OSX/Windows Pd 0.42.5 vanilla and extended Tcl/Tk 8.4 & 8.5

In addition to the detailed description provided in the previous email, here's a list of fixes:

*redesigned scrolling algorithm so that is no 0 0 centric but rather viewport-centric (e.g. if everything can fit within the said window, it should)

*100% accurate bbox creation including text which requires font resizing workaround (known tcl/tk issue). Font resizing has to be done per-font (Linux fix provided for three default fonts, currently other OSs use old bbox style even though it is broken mainly because its fonts need to be manually re-calibrated). Font resizing has been tested accurate up to 1000px-sized fonts but should work beyond as well. NB: this has been mainly tested with 8.5 with antialiased fonts on Linux but should work on other versions as well. The text also avoids calculating bbox for text entries that are empty (something that existing version does making its bbox incorrect and rendering GUI creation with tight window margins impossible, e.g. a large number2 object with a window size matching object's)

*scroll wheel scrolls only if the scrollbars are active (back-ported from pd-extended)

*scrollbars update automatically and dynamically with undo/redo/cut/paste/resize/typing text, as well as apply actions in canvas and object properties where the change may require creation or hiding of scrollbars (e.g. creating an atom and then in its properties applying width of 1000 which then spills outside the visible window will automatically update scrollbars).

*array and graph are spawned always on-screen regardless how big patch is or how far it is currently located away from the 0 0 origin.

*array name is immediately displayed at creation-time rather than requiring moving the object before it is properly displayed.

*properties windows spawn right next to the mouse which makes editing a lot easier rather than hunting the widget across the random desktop spawning locations.

Now I think this should pass all Hans's pd-gui-rewrite tests plus provide a few more test cases none of the existing models account for.

Attached tarball consists of 15 separate patches (named after the file they are meant to patch) and should patch cleanly against 0.42.5 pd-extended source. Changes should be also mostly self-explanatory or are commented accordingly.

Cheers!

ico <bigkahuna_100519.tar.gz>

----------------------------------------------------------------------------

You can't steal a gift. Bird gave the world his music, and if you can hear it, you can have it. - Dizzy Gillespie