The current dialog should really be replaced by something more intuitive and less "rigid" feeling at some point. I personally like the way they do the compositor nodes in Blender and think that perhaps something more like that could be easier to work with.

Here's one screenshot with it for those who aren't familiar. http://www.blender.org/typo3temp/pics/61389a332f.jpg

Cheers, Josh

On Tue, 2010-07-27 at 01:50 +0200, Krzysztof Kosiński wrote:

Additional problem: there is no horizontal scrolling in the connection editor widget, so with large filters (e.g. 20 primitives or more) the dialog gets very wide.

Regards, Krzysztof

---

The Palm PDK Hot Apps Program offers developers who use the Plug-In Development Kit to bring their C/C++ apps to Palm for a share of $1 Million in cash or HP Products. Visit us here for more details: http://ad.doubleclick.net/clk;226879339;13503038;l? http://clk.atdmt.com/CRS/go/247765532/direct/01/ _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel

On 7/27/10, Krzysztof Kosiński wrote:

W dniu 27 lipca 2010 02:04 użytkownik Joshua A. Andler <scislac@...400...> napisał:

...

Here's one screenshot with it for those who aren't familiar. http://www.blender.org/typo3temp/pics/61389a332f.jpg

Wow, this is really nice. We would need a really complex custom widget though.

Actually we have at least two bluepapers exactly on alternative UI for SVG filters: one suggests a simpler UI and the other one goes for Blender-like UI.

We don't exactly have to write our own widget from scratch. MathMap (http://www.complang.tuwien.ac.at/schani/mathmap/) already has one (GTK+, C, Cairo) which is quite similar to the one from Quartz Composer, even though it doesn't have per-node preview to the best of my knowledge.

Alexandre Prokoudine http://libregraphicsworld.org

W dniu 27 lipca 2010 10:32 użytkownik Tavmjong Bah <tavmjong@...8...> napisał:

It is a problem with our current interface that you cannot allow all reasonable values and have the slider cover the potentially most used range. Currently, the only way you can set values outside the slider/entry box range is to use the XML editor so it is better to error on the side of too large ranges than too small.

It would be best if the slider had a limited range, but you could input an arbitrary value (or a value from a much larger range) using the spinbox.

579932 Too much states saved for undo/redo when using sliders https://bugs.launchpad.net/inkscape/+bug/579932

The easiest fix is to set the sliders to Gtk::UPDATE_DISCONTINUOUS update policy, but this will prevent the display from updating during the drag. Apparently there is no way in GTK right now to get the correct behavior!

Regards, Krzysztof

2010/7/27 Krzysztof Kosiński <tweenk.pl@...400...>:

The easiest fix is to set the sliders to Gtk::UPDATE_DISCONTINUOUS update policy, but this will prevent the display from updating during the drag. Apparently there is no way in GTK right now to get the correct behavior!

There is. See how the color sliders in Fill&Stroke deal with this. They also update repr continuously during drag, which enables live display update, but all these updates are committed with the same undo key. As a result, when you undo, the entire drag is undone. Once you release, the key is flipped to another value so the next drag is not lumped together with previous one.

On Jul 27, 2010, at 3:35 PM, Krzysztof Kosiński wrote:

It turns out that the author of the dialog was thinking correctly but did not know that some Inkscape APIs were braindead. The undo spam was triggered because the undo key passed to sp_document_maybe_done was dynamically created in a Glib::ustring, and that function expects a constant string. I fixed it to store a duplicate of the last undo action key in SPDocument (via g_strdup), rather than simply assigning a char pointer. The destructor was adjusted as well.

Good catch.

There are a few things about the code, though. First of all the API was not "braindead", there was just an improper bit of implementation code. But more important is that the fix needs a few things. First of all is g_strdup(). We really need to avoid having that in any C++ code. Next is that you had an if statement without any braces around its body.

Aside from those code safety points, though, the fix looks good and we probably want a cleaned up version going into 0.48.

On Jul 27, 2010, at 9:52 PM, bulia byak wrote:

Why do you think that this is where this has to be fixed, instead of switching the filters dialog to static strings? Fill&stroke uses static strings just fine.

I don't see any necessity for dynamic generation of keys, and doing a string dup on each undo event sounds unnecessarily wasteful.

In any case, this is a nontrivial change in a very sensitive area, so I would prefer to err on the side of caution and not apply it to 0.48, given that the problem it fixes is relatively cosmetic.

I think Krzysztof might have miscommunicated some of what he was seeing. From his email I also got the same impression, but once I looked at the actual diff it was clear there were problems in the implementation of sp_document_maybe_done().

It's not such much that dynamic generation of keys was the main issue, but that SPDocument was holding onto a temporary pointer retuned from Glib::ustring, which in turn is just std::string::c_str().

So the problem comes up that later on when we go to compare things to the next string, the pointer is invalid and could be pointing anywhere. Holding stale pointers can easily cause crashes. Some OS's are also more susceptible to these.. Erring on the side of caution would probably say not to hold stale pointers.

If we're really concerned about performance, we should take a look at passing in a reference to a Glib::ustring. The C++ classes usually have been optimized to share buffers, etc. Of course we could probably search through all the calls to sp_document_maybe_done() and ensure they only use static persistent strings, but we still should look for ways to keep the code safe.

2010/7/28 Krzysztof Kosiński <tweenk.pl@...400...>:

W dniu 28 lipca 2010 06:52 użytkownik bulia byak <buliabyak@...400...> napisał:

...

I don't see any necessity for dynamic generation of keys, and doing a string dup on each undo event sounds unnecessarily wasteful.

~50 bytes of extra memory consumption per document looks like a reasonable price to pay to fix undo in the filter dialog, and possible future places that will need to use dynamic generation of undo keys.

Once again: we don't need dynamic generation of keys. Look a fill&stroke: it just flips between two static strings. It's faster and easier.

It's also much lower than the memory required to store all the static strings. Only the last undo key is stored.

If you're counting static memory, then I'm quite certain that the extra ustring calls and related overhead will easily eat those several bytes of economy you get from eliminating two static strings. On top of which, ustring also allocates dynamic memory, while static strings do not.

W dniu 28 lipca 2010 18:15 użytkownik bulia byak <buliabyak@...400...> napisał:

2010/7/28 Krzysztof Kosiński <tweenk.pl@...400...>:

...

W dniu 28 lipca 2010 06:52 użytkownik bulia byak <buliabyak@...972.....> napisał:

...

I don't see any necessity for dynamic generation of keys, and doing a string dup on each undo event sounds unnecessarily wasteful.

~50 bytes of extra memory consumption per document looks like a reasonable price to pay to fix undo in the filter dialog, and possible future places that will need to use dynamic generation of undo keys.

Once again: we don't need dynamic generation of keys. Look a fill&stroke: it just flips between two static strings. It's faster and easier.

Here's why we can't use static strings in the filter dialog:

Let's say you have two Color Matrix primitives with type Hue Rotate, adjust one of them, then select the second one and adjust it as well. The two events should not be coalesced, which means they should have different undo keys. Since there is no limit on the number of Color Matrix primitives in a filter, we must generate the undo key string dynamically.

Of course you could hack around it by storing the undo key string in the filter dialog and changing the argument to maybe_done between e.g. "filter:1" and "filter:2" whenever the actual undo key string changes, but doesn't it make more sense to fix the API limitation instead of papering over it?

PS: This might be a small inefficiency, but it uses much simpler code than alternative solutions.

Regards, Krzysztof