Hello all
Today I have implemented snapping of nodes, so the new node tool should be ready to go into trunk. As a reminder, here is the status table: http://wiki.inkscape.org/wiki/index.php/GSoC2009_Node_Tool_Rewrite
There are still some minor issues with snapping: 1. Nodes do not snap to unselected segments of selected paths. 2. Nodes do not snap to selected shapes. They will be fixed once I refactor the event context / tool hierarchy, which I plan to do before this year's GSoC.
Now I have a few remarks / questions about the behavior:
1. There is an RFE: https://bugs.launchpad.net/inkscape/+bug/321150 The proposed is like the new node tool's join action, but with a distance threshold. Implementing this should be easy, but I don't know how to present it in the UI. Ideas, anyone?
2. Releasing the mouse button causes the point to immediately snap, even if the snap delay has not expired. For me, this unexpected and very annoying: I have to turn off snapping to make a non-snapped movement. If this feature was removed, I could comfortably make both snapped and non-snapped movements with snapping enabled. Currently I disabled snapping on mouse button release.
3. When a point snaps to a path, it should remain snapped to it until the pointer wanders outside the snap distance. This would allow me to adjust the point's position on a path it snapped to, without waiting for the snap delay to expire every time. Same behavior for snapping to nodes, corenrs, etc. (the point remains snapped until I move the mouse pointer out of the snapping range). What do you think?
4. The method to switch to the transform handles to rotation mode is very non-obvious. Currently it is Shift+R. To be consistent with the selector, the method to switch to rotation mode should be to click on a selected node, but this might become annoying quickly - the old behavior is to select only the clicked node in this case. I think the second behavior is acceptable if it only works when transform handles are shown, and the handles can be toggled using a button on the controls bar (preferably the first one).
Regards, Krzysztof.
On Sun, 2010-01-10 at 03:06 +0100, Krzysztof Kosiński wrote:
Hello all
Today I have implemented snapping of nodes, so the new node tool should be ready to go into trunk.
Was this affected by Diedrik's changes today?
As a reminder, here is the status table: http://wiki.inkscape.org/wiki/index.php/GSoC2009_Node_Tool_Rewrite
So sculpting still does not work?
There are still some minor issues with snapping:
- Nodes do not snap to unselected segments of selected paths.
- Nodes do not snap to selected shapes.
They will be fixed once I refactor the event context / tool hierarchy, which I plan to do before this year's GSoC.
What does this year's GSoC (hopefully there is one) have to do with when this will be done? Was that just an "it should be done by summer" comment?
- The method to switch to the transform handles to rotation mode is
very non-obvious. Currently it is Shift+R. To be consistent with the selector, the method to switch to rotation mode should be to click on a selected node, but this might become annoying quickly - the old behavior is to select only the clicked node in this case. I think the second behavior is acceptable if it only works when transform handles are shown, and the handles can be toggled using a button on the controls bar (preferably the first one).
Agreed that this is touchy territory. A key combo seems bad as it lacks the consistency we typically have in our UI between tools and behavior. I don't think it would become too terribly annoying for clicking between modes since it would be consistent with the behavior of the Selector Tool (for me at least). As for the handles being toggled via the Controls Bar, is the thought that it's an on/off switch (if a selection of >1 node, always a show queue) or a one time toggle (click it to toggle on for the current selection only)?
I look forward to this being merged! :)
Cheers, Josh
W dniu 10 stycznia 2010 04:27 użytkownik Joshua A. Andler <scislac@...400...> napisał:
On Sun, 2010-01-10 at 03:06 +0100, Krzysztof Kosiński wrote:
Hello all
Today I have implemented snapping of nodes, so the new node tool should be ready to go into trunk.
Was this affected by Diedrik's changes today?
Apparently, yes :/
As a reminder, here is the status table: http://wiki.inkscape.org/wiki/index.php/GSoC2009_Node_Tool_Rewrite
So sculpting still does not work?
Yes, it doesn't work yet. If it's a must before it goes into trunk, then of course I will implement it, but without the ability to change "sculpting profiles" (there was no UI for this before as well). They will be back once I add enumerations as a value type to the preferences API.
What does this year's GSoC (hopefully there is one) have to do with when this will be done? Was that just an "it should be done by summer" comment?
It is a promise that means "I will fix this before I start a new large project" :)
As for the handles being toggled via the Controls Bar, is the thought that it's an on/off switch (if a selection of >1 node, always a show queue) or a one time toggle (click it to toggle on for the current selection only)?
It should be an on/off switch. It would still possible to clear the node selection by clicking on a node that's not selected, or in an empty space. When working with transforms, constantly clicking the button after each selection change could become annoying as well :) What does it mean "(if a selection of >1 node, always a show queue)"? Right now the default is to show the handles only when more than one node is selected, but there is an option to turn it on even for single nodes.
Regards, Krzysztof
I think it should be merged now, whatever bugs are left and sculpting can be finished in the trunk.
2010/1/9 Joshua A. Andler <scislac@...400...>:
I look forward to this being merged! :)
2010/1/9 Krzysztof Kosiński <tweenk.pl@...400...>:
- There is an RFE: https://bugs.launchpad.net/inkscape/+bug/321150
The proposed is like the new node tool's join action, but with a distance threshold. Implementing this should be easy, but I don't know how to present it in the UI. Ideas, anyone?
This sounds like a command for Path menu, not something specific to Node tool. Maybe also combine it with other cleanup operations, such as removing subpaths with zero or one node, and call "Clean up path". Its promise will be to preserve appearance for solid paths but possibly change it if there are markers or dashes.
- The method to switch to the transform handles to rotation mode is
very non-obvious. Currently it is Shift+R.
As we discussed, in Selector it's Shift+S, but we cannot use the same in Node because there it means turning node to smooth. We also shouldn't use Shift+R because it conflicts with reversing path (analogous to reversing gradient in Gradient tool). We need to use some globally unused, and ideally somewhat intuitive, Shift+letter combination. In addition to that we might add a toggle button for this, on Node as well as Selector controls bars.
W dniu 10 stycznia 2010 05:20 użytkownik bulia byak <buliabyak@...400...> napisał:
This sounds like a command for Path menu, not something specific to Node tool. Maybe also combine it with other cleanup operations, such as removing subpaths with zero or one node, and call "Clean up path". Its promise will be to preserve appearance for solid paths but possibly change it if there are markers or dashes.
Right now it is not possible to create zero-node subpaths. Single-node subpaths can exist only when they have at least one bezier handle extended. The clean up is performed after every action that can create such degenerate subpaths: deletion, break, etc. so I don't think this needs to be put in a separate action.
However, I found a different solution. The join action should at first only join endnodes, and if no joins can be made, it should weld contiguous stretches of the selection into single nodes. Same could be done for segment join. This way selecting all nodes in a path and performing the join action will first join all endnodes according to a pairwise nearest neighbor algorithm, and then do nothing (because when all nodes are selected, then it does nothing by design - removing the path in such a case would be surprising) - exactly what is needed for that RFE.
Regards, Krzysztof
On 01/10/2010 03:06 AM, Krzysztof Kosiński wrote:
Today I have implemented snapping of nodes, so the new node tool should be ready to go into trunk.
I've just made some changes to the snapping API today, as Joshua already noted. Shouldn't take you more than a few minutes extra though :-)
- Releasing the mouse button causes the point to immediately snap,
even if the snap delay has not expired. For me, this unexpected and very annoying: I have to turn off snapping to make a non-snapped movement.
Well, that's the whole idea of the snapping mechanism: If snapping is on, then snapping should take place. For some tools though it can be overridden by holding the shift key. But maybe I don't understand the problem here; what else would you expect? BTW, there is a shortcut key to toggle snapping quickly.
- When a point snaps to a path, it should remain snapped to it until
the pointer wanders outside the snap distance. This would allow me to adjust the point's position on a path it snapped to, without waiting for the snap delay to expire every time.
Why don't you just turn off the snap delay in the preferences? This gives you the behavior you're looking for, or doesn't it?
Regards,
Diederik
[ warning: thread-high-jacking ;) ]
On 10/1/10 09:00, Diederik van Lierop wrote:
On 01/10/2010 03:06 AM, Krzysztof Kosiński wrote:
Today I have implemented snapping of nodes, so the new node tool should be ready to go into trunk.
I've just made some changes to the snapping API today, as Joshua already noted. Shouldn't take you more than a few minutes extra though :-)
rev. 8960 is very verbose ;) when compiling on osx [1]: 580 times
./snap-candidate.h:116: warning: unused parameter ‘_additional_affine’
builds fine though and so far not I haven't seen any issues when using snapping (didn't test with transformed masked or clipped objects yet).
~suv
[1] i686-apple-darwin9-g++-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493)
build flags from configure:
Compiler: ccache g++ CPPFLAGS: -Wall -Wformat -Wformat-security -W -D_FORTIFY_SOURCE=2 -I/Volumes/blue/mp/include CXXFLAGS: -Wpointer-arith -Wcast-align -Wsign-compare -Woverloaded-virtual -Wswitch -Wno-unused-parameter -O3 -Wall CFLAGS: -Wno-pointer-sign -O3 -Wall LDFLAGS: -L/Volumes/blue/mp/lib
On 10/1/10 11:11, ~suv wrote:
[ warning: thread-high-jacking ;) ]
On 10/1/10 09:00, Diederik van Lierop wrote:
On 01/10/2010 03:06 AM, Krzysztof Kosiński wrote:
Today I have implemented snapping of nodes, so the new node tool should be ready to go into trunk.
I've just made some changes to the snapping API today, as Joshua already noted. Shouldn't take you more than a few minutes extra though :-)
rev. 8960 is very verbose ;) when compiling on osx [1]: 580 times
./snap-candidate.h:116: warning: unused parameter ‘_additional_affine’
builds fine though and so far not I haven't seen any issues when using snapping (didn't test with transformed masked or clipped objects yet).
oops, not true - e.g. node snapping 'Snap to path' and 'Snap to path intersections' fails for me, works with r8958 (tested with default prefs in new file from default A4 template).
~suv
On 01/10/2010 11:42 AM, ~suv wrote:
oops, not true - e.g. node snapping 'Snap to path' and 'Snap to path intersections' fails for me, works with r8958 (tested with default prefs in new file from default A4 template).
~suv
This has been fixed by my previous patch too, I just verified that. Removing an underscore was all that it took :-)
Diederik
On 11/1/10 17:40, Diederik van Lierop wrote:
On 01/10/2010 11:42 AM, ~suv wrote:
oops, not true - e.g. node snapping 'Snap to path' and 'Snap to path intersections' fails for me, works with r8958 (tested with default prefs in new file from default A4 template).
This has been fixed by my previous patch too, I just verified that. Removing an underscore was all that it took :-)
Yep, thanks for fixing that. I should have confirmed earlier that fixing the typo also restored both mentioned snapping modes, sorry.
~suv
On Jan 10, 2010, at 12:00 AM, Diederik van Lierop wrote:
I've just made some changes to the snapping API today, as Joshua already noted. Shouldn't take you more than a few minutes extra though :-)
Can you take a quick peek? It looks like snap-candidate.h has an error in the initializer in line 116.
SnapCandidateItem(SPItem* item, bool clip_or_mask, Geom::Matrix _additional_affine) : item(item), clip_or_mask(clip_or_mask), additional_affine(additional_affine) {}
I think the parameter needs to get rid of that extra underscore.
Also.. for the initialization, consider keeping that on separate lines. Makes it a little more readable, and also makes diff's easier to view over time. It also makes things easier to breakpoint and walk in the debugger:
SnapCandidateItem(SPItem* item, bool clip_or_mask, Geom::Matrix _additional_affine) : item(item), clip_or_mask(clip_or_mask), additional_affine(additional_affine) {}
Thanks.
On 01/10/2010 12:09 PM, Jon Cruz wrote:
Can you take a quick peek? It looks like snap-candidate.h has an error in the initializer in line 116.
Fixed. But is the crash below yours?
Program received signal SIGSEGV, Segmentation fault. 0x000000362c87ecd2 in __strlen_sse2 () from /lib64/libc.so.6
(gdb) bt #0 0x000000362c87ecd2 in __strlen_sse2 () from /lib64/libc.so.6 #1 0x000000300ae590e2 in g_strdup () from /lib64/libglib-2.0.so.0 #2 0x000000300b23107d in ?? () from /lib64/libgobject-2.0.so.0 #3 0x0000003fe0d49f32 in ?? () from /usr/lib64/libgtk-x11-2.0.so.0 #4 0x0000003fe0d4a419 in gtk_list_store_set_valist () from /usr/lib64/libgtk-x11-2.0.so.0 #5 0x0000003fe0d4a518 in gtk_list_store_set () from /usr/lib64/libgtk-x11-2.0.so.0 #6 0x00000000007e91e8 in create_or_fetch_actions (desktop=0x1a51c00) at widgets/toolbox.cpp:849 #7 0x00000000007ec5d9 in setupToolboxCommon (toolbox=0x2beb1c0, desktop=0x1a51c00, descr= 0xe25198 "<ui> <toolbar name='ToolToolbar'> <toolitem action='ToolSelector' /> <toolitem action='ToolNode' /> <toolitem action='ToolTweak' /> <toolitem action='ToolSpray' /> <toolitem action='To"..., toolbarName=0xe25478 "/ui/ToolToolbar", sizePref=0xe2487e "/toolbox/tools/small") at widgets/toolbox.cpp:1631 #8 0x00000000007ecf0d in setup_tool_toolbox (toolbox=0x2beb1c0, desktop=0x1a51c00) at widgets/toolbox.cpp:1758 #9 0x00000000007ec474 in Inkscape::UI::ToolboxFactory::setToolboxDesktop (toolbox=0x2beb1c0, desktop=0x1a51c00) at widgets/toolbox.cpp:1615 #10 0x0000000000833888 in Inkscape::UI::UXManager::connectToDesktop (this=0x2daffd0, toolboxes=std::vector of length 4, capacity 4 = {...}, desktop=0x1a51c00) at ui/uxmanager.cpp:98 #11 0x00000000007b5ce8 in SPDesktopWidget::createInstance (namedview=0x22680a0) at widgets/desktop-widget.cpp:1395 #12 0x00000000007b5904 in sp_desktop_widget_new (namedview=0x22680a0) at widgets/desktop-widget.cpp:1346 #13 0x00000000004649f8 in sp_file_new (templ=...) at file.cpp:129 #14 0x0000000000464d4d in sp_file_new_default () at file.cpp:177 #15 0x000000000044d5b4 in sp_main_gui (argc=1, argv=0x7fffffffe1a8) at main.cpp:949 #16 0x00000000005abe32 in Inkscape::NSApplication::Application::run (this=0x7fffffffe070) at application/application.cpp:114 #17 0x000000000044ca4a in main (argc=1, argv=0x7fffffffe1a8) at main.cpp:689
Regards,
Diederik
2010/1/10 Diederik van Lierop <mail@...1689...>:
On 01/10/2010 12:09 PM, Jon Cruz wrote:
Can you take a quick peek? It looks like snap-candidate.h has an error in the initializer in line 116.
Fixed. But is the crash below yours?
No, this one is related to Jon's "UXManager" in src/ui/uxmanager.cpp. I don't know what it does, but I presume it's a stub that will be fleshed out later.
Jon, if this is a refactoring effort, can it be relocated to a separate branch until 0.48? I thought that refactoring is discouraged until the 0.49 cycle.
Regards, Krzysztof
On Jan 10, 2010, at 10:38 AM, Krzysztof Kosiński wrote:
W dniu 10 stycznia 2010 19:30 użytkownik Jon Cruz <jon@...18...> napisał:
No, it's not.
Please note Josh's mail on 0.48
OK, I see it now. You are right, it doesn't mention a ban on refactoring. Regards, Krzysztof
No, no. You missed the point.
This is not refactoring, this is the new feature that Josh mentioned: "*JonCruz's Adaptive UI work"
On Sun, 2010-01-10 at 19:38 +0100, Krzysztof Kosiński wrote:
W dniu 10 stycznia 2010 19:30 użytkownik Jon Cruz <jon@...18...> napisał:
No, it's not.
Please note Josh's mail on 0.48
OK, I see it now. You are right, it doesn't mention a ban on refactoring.
Just to note, there is never a ban on refactoring. It's always preferred for anything that has been refactored (or any major changes for that matter) to be committed at the beginning of a dev cycle so it can get proper testing and fixing as necessary.
Regarding the Node Tool work, if bulia endorses a merge, I say to go for it. More time for testing and feedback is a good thing. I hope though that you will have time to fix bugs should people find and file them.
Cheers, Josh
On Jan 10, 2010, at 6:17 AM, Krzysztof Kosiński wrote:
2010/1/10 Diederik van Lierop <mail@...1689...>:
On 01/10/2010 12:09 PM, Jon Cruz wrote:
Can you take a quick peek? It looks like snap-candidate.h has an error in the initializer in line 116.
Fixed. But is the crash below yours?
No, this one is related to Jon's "UXManager" in src/ui/uxmanager.cpp. I don't know what it does, but I presume it's a stub that will be fleshed out later.
Well, that should have been "Yes", as Diederik was replying to and asking me. :-)
W dniu 10 stycznia 2010 09:00 użytkownik Diederik van Lierop <mail@...1689...> napisał:
Well, that's the whole idea of the snapping mechanism: If snapping is on, then snapping should take place. For some tools though it can be overridden by holding the shift key. But maybe I don't understand the problem here; what else would you expect? BTW, there is a shortcut key to toggle snapping quickly.
I would expect that if there is no snap indicator displayed, the point I'm dragging will stay in the place it's currently at when I drop it, according to the principle of least astonishment. If there is no indication of snapping, snapping should not occur. Otherwise the snap delay setting is not useful: my movements will snap regardless of whether the snap indicator is displayed or not, so there is no reason to slow down work by setting it to something larger than 0.
Why don't you just turn off the snap delay in the preferences? This gives you the behavior you're looking for, or doesn't it?
It does, but only sort of. If the snap delay is zero, I can no longer make unsnapped movements. If the snap delay is non-zero, I have to wait for the snapping delay to expire every time I adjust the position. So there is no way for me to win. :) I know this might not be easy to implement, but after playing with snapping for some time I think such behavior would be the most convenient for the user.
Regards, Krzysztof
On 01/10/2010 03:36 PM, Krzysztof Kosiński wrote:
I would expect that if there is no snap indicator displayed, the point I'm dragging will stay in the place it's currently at when I drop it, according to the principle of least astonishment. If there is no indication of snapping, snapping should not occur.
Ah, now I see: you're interpreting the snap indicator differently. If you see the snap indicator then snapping _has_ already occurred. The indicator has been implemented to show what we've snapped to, not to show what we'll snap to _after_ the mouse button is released.
Otherwise the snap delay setting is not useful: my movements will snap regardless of whether the snap indicator is displayed or not,
That's how it's supposed to work. Forget the snap indicator for a moment, and think about snapping to a grid. When using a grid, many users will expect all objects to be aligned to any of the grid lines, at any time; after all that's what you're using a grid for. Now when dragging an object it will become unaligned, but it should be aligned again once its released (only if snapping has been enabled ofcourse). That's why snapping is triggered when the mouse button is released. The snap delay mechanism controls how often Inkscape tries to snap while dragging the object around, but still we always snap when releasing the mouse button. The snap indicator only tells us where snapping took place.
Why don't you just turn off the snap delay in the preferences? This gives you the behavior you're looking for, or doesn't it?
It does, but only sort of. If the snap delay is zero, I can no longer make unsnapped movements.
Believe me, there are users who prefer it like this! I found that out _after_ implementing the snap delay ;-)
If the snap delay is non-zero, I have to wait for the snapping delay to expire every time I adjust the position. So there is no way for me to win. :)
Correct, you'll have to switch snapping off yourself explicitly, Inkscape will not do that for you. We should not try to guess when a user wants to snap and when not, we might guess wrong without the user noticing.
The most elegant way is to disable snapping when holding the SHIFT key, as some tools do, but unfortunately the modifier keys are already heavily contested so we will probably never get that implemented in all tools. The second best thing is to use the % key to toggle snapping, and to toggle it back on again afterwards.
Regards,
Diederik
W dniu 10 stycznia 2010 21:13 użytkownik Diederik van Lierop <mail@...1689...> napisał:
On 01/10/2010 03:36 PM, Krzysztof Kosiński wrote: Ah, now I see: you're interpreting the snap indicator differently. If you see the snap indicator then snapping _has_ already occurred. The indicator has been implemented to show what we've snapped to, not to show what we'll snap to _after_ the mouse button is released.
So we are not showing what will actually happen, or are only showing it after some delay which interferes with normal work. If the majority really wants to use the snap delay only as an optimization, then we should set the snap delay to something low (like 50 ms) and remove it from the UI, because it has absolutely no effect for the user.
Now when dragging an object it will become unaligned, but it should be aligned again once its released (only if snapping has been enabled ofcourse). That's why snapping is triggered when the mouse button is released.
That's circular. Why should it become aligned again if I dragged it, especially if there is no indication of that happening in the UI?
The snap delay mechanism controls how often Inkscape tries to snap while dragging the object around, but still we always snap when releasing the mouse button. The snap indicator only tells us where snapping took place.
Yes, I know but you didn't give any explanation why this is the best behavior. I think it is counter-intuitive, based on the assumption that when snapping is enabled, we should always snap. If this assumption is true, the snap delay is redundant and gets in the way of efficient work, because we hold back information from the user.
Believe me, there are users who prefer it like this! I found that out _after_ implementing the snap delay ;-)
Maybe they did not want the snap delay at all? The current behavior is really making this setting useless. It only affects how much time you need to waste to see what will happen after you release the mouse, so setting it to something else than 0 is counter-productive (unless you have a very big document and snapping is resource-intensive - then you can treat is as a kind of optimization).
If someone wants his movements to always snap, he should set the snap delay to zero, instead of making this setting useless for everyone else. If the majority really wants such behavior, then I think the snap delay setting should be reduced to about 50 ms and removed from the UI, as mentioned above.
Correct, you'll have to switch snapping off yourself explicitly, Inkscape will not do that for you. We should not try to guess when a user wants to snap and when not, we might guess wrong without the user noticing.
If I did not move the pointer for the last 300 ms or so, then it means I want to snap. If I did, then probably I'm making a movement that shouldn't snap. If I wanted different behavior, I would adjust the snap delay setting. When we snap on release, we are guessing that the user wants to snap, but this guess is not based on any input. It is just an assumption pulled out from nowhere. In fact we actively ignore the user's preferred value of snap delay. If we only snap after the snap delay expires, our guess is based on somewhat explicit input from the user - he didn't move the mouse.
If the majority wants to snap most of the time, we can set the default snap delay to something low (a few ms). This way we satisfy the always-snap group (they won't notice the delay), get most of the optimization benefits and don't make this setting useless for the rest (like me).
The most elegant way is to disable snapping when holding the SHIFT key, as some tools do, but unfortunately the modifier keys are already heavily contested so we will probably never get that implemented in all tools. The second best thing is to use the % key to toggle snapping, and to toggle it back on again afterwards.
I could turn snapping on and off every time, or use the already overloaded modifier keys. But I could as well tweak the snap delay to my liking and work more efficiently, without touching the keyboard - that's the point :)
This could be improved even further by implementing my second idea: when something snaps, it should remain snapped to the same snap target until the pointer moves out of range. This way minor accidental movements after the snap delay expires do not break the snap - I have to make a larger movement to move away the point. This way most of the user-side rationale for always snapping on release (it's easy to accidentally move the mouse a bit while releasing the button) would disappear.
For point snaptargets it's very simple to implement: after we snapped to a point, do not move until the point's new position would be more than the snap distance away from the current one. For paths and lines it's a bit more involved. After the initial snap we want to only move the point on the snapped line or curve. We would need to store some information about the snap target, for example the geometrical representation the snapped shape segment.
Regards, Krzysztof
On 01/10/2010 10:43 PM, Krzysztof Kosiński wrote:
So we are not showing what will actually happen, or are only showing it after some delay which interferes with normal work. If the majority really wants to use the snap delay only as an optimization, then we should set the snap delay to something low (like 50 ms) and remove it from the UI, because it has absolutely no effect for the user.
It's in the UI because I didn't know at that time what a good value would be, and to allow users to turn it off and get the old behavior back. The latter might still be desired, so I propose leaving the slider there, but we can indeed lower the default value as you suggested. Could you do a short test and see at what value it becomes acceptable for you?
Now when dragging an object it will become unaligned, but it should be aligned again once its released (only if snapping has been enabled ofcourse). That's why snapping is triggered when the mouse button is released.
That's circular. Why should it become aligned again if I dragged it, especially if there is no indication of that happening in the UI?
My assumption here is that the user is moving from one aligned position to the other, and the unaligned position is just an intermediate state. So if the user drags an object it's not because he wants it to become unaligned, but to move to another aligned position. Different users might have different expectations though.
The snap delay mechanism controls how often Inkscape tries to snap while dragging the object around, but still we always snap when releasing the mouse button. The snap indicator only tells us where snapping took place.
Yes, I know but you didn't give any explanation why this is the best behavior. I think it is counter-intuitive, based on the assumption that when snapping is enabled, we should always snap.
It might be counter-intuitive to you, but at least it's predictable. IMHO that's very important for users who use snapping because they tend to value accuracy and want to be sure that their drawings are perfectly aligned and orthogonal. In the days before we had the snap-indicator this predictability was even more important because they didn't get any feedback; nowadays this predictability is still useful though.
Believe me, there are users who prefer it like this! I found that out _after_ implementing the snap delay ;-)
Maybe they did not want the snap delay at all?
That could very well be the case, so that's why they can disable it. You will need it though on complex documents, or on a slow pc
The current behavior is really making this setting useless. It only affects how much time you need to waste to see what will happen after you release the mouse, so setting it to something else than 0 is counter-productive
For me 150 msec (the current default value if I'm correct) is hardly noticeable, but maybe I'm getting old ;-) ? Please confirm if you really want to go as low as 50 msec. Have a try at e.g. 75 or 100 msec and report your experiences. I think the higher the more better, but obviously there should be an optimum somewhere.
If I did not move the pointer for the last 300 ms or so, then it means I want to snap. If I did, then probably I'm making a movement that shouldn't snap. If I wanted different behavior, I would adjust the snap delay setting. When we snap on release, we are guessing that the user wants to snap
No we're not guessing here, we're being predictable! Snapping is enabled, so we'll snap. Always. Period.
This could be improved even further by implementing my second idea: when something snaps, it should remain snapped to the same snap target until the pointer moves out of range.
... or until it comes closer to another snap target. This is exactly the behavior you'll get when you set the snap delay to zero. This requires some snapping code to be executed for each and every motion event.
This way minor accidental movements after the snap delay expires do not break the snap - I have to make a larger movement to move away the point. This way most of the user-side rationale for always snapping on release (it's easy to accidentally move the mouse a bit while releasing the button) would disappear.
Because of the boundary condition that snapping should always occur, and because we don't want to snap on each and every motion event (the snap delay mechanism), we must snap when releasing the mouse button. If the user drops an object "on the fly", snapping must be triggered, even though the mouse speed doesn't drop to zero. So only monitoring the speed of the pointer to trigger snapping is not sufficient.
Diederik
W dniu 11 stycznia 2010 22:11 użytkownik Diederik van Lierop <mail@...1689...> napisał:
It might be counter-intuitive to you, but at least it's predictable. IMHO that's very important for users who use snapping because they tend to value accuracy and want to be sure that their drawings are perfectly aligned and orthogonal. In the days before we had the snap-indicator this predictability was even more important because they didn't get any feedback; nowadays this predictability is still useful though.
This behavior might be predictable, but it is surprising. The argument that some behavior is predictable is weak, because almost all programs (even viruses) behave in a predictable way, unless they are broken or purposefully random. If something is predictable, it doesn't mean it's right, intuitive or convenient. Also note that the suggested new behavior is also trivially predictable: only snap when a snap indicator is present.
The first time I encountered it, it was like this: 1. I move the node near another node, with snapping enabled. 2. I release the mouse button, and there is no snap indicator. I think it should stay where I drop it. 3. Bang! It moves to the other node's position. What happen?!? There was no snap indicator, so I expected the node to stay where it is.
Adding the snap indicator changed the game fundamentally. When there was no snap indicator, the only information the user had was that snapping is enabled. But with the snap indicator present, we notify him when we'll snap. So why do we sometimes snap without notifying him?
No we're not guessing here, we're being predictable! Snapping is enabled, so we'll snap. Always. Period.
Snapping is enabled, but the snap delay is enabled as well! So apparently the user doesn't want to snap right away, otherwise he would decrease or disable it...
This could be improved even further by implementing my second idea: when something snaps, it should remain snapped to the same snap target until the pointer moves out of range.
... or until it comes closer to another snap target. This is exactly the behavior you'll get when you set the snap delay to zero. This requires some snapping code to be executed for each and every motion event.
Not quite. The whole point is to "stick" only to the target we snapped to, and ignore other snap targets. To snap to another target, the user has to move the pointer out of range and wait for the snap delay to expire again. This way it's harder for people that don't have a perfectly steady hand to snap to something they didn't want to if they are working in a crowded area of the drawing.
To get the behavior I suggested, I only need to either: a) check whether the distance from the snapped point would be larger than the snap distance, if I snapped to a point. b) compute the nearest point for the path fragment, and check whether it's farther away from the new position than the snap distance, if I snapped to a path. Those computations are trivial and cannot impact performance in any significant way. There's already some processing done on each motion event. Adding this would be insignificant.
Regards, Krzysztof
On Tue, 12 Jan 2010 05:46:30 +0100, Krzysztof Kosiński <tweenk.pl@...400...> wrote:
The argument that some behavior is predictable is weak, because almost all programs (even viruses) behave in a predictable way, unless they are broken or purposefully random. If something is predictable, it doesn't mean it's right, intuitive or convenient.
It's the other way around: something that is _not_ predictable is bad. I'm obviously not arguing that something that's predictable is always good, regardless of the implementation
Also note that the suggested new behavior is also trivially predictable: only snap when a snap indicator is present.
No, because the snap indicator can be overlooked or can be disabled. And if that happens the predictability is gone.
The first time I encountered it, it was like this:
- I move the node near another node, with snapping enabled.
- I release the mouse button, and there is no snap indicator. I think
it should stay where I drop it. 3. Bang! It moves to the other node's position. What happen?!? There was no snap indicator, so I expected the node to stay where it is.
Reduce the snap delay and this will be solved.
Adding the snap indicator changed the game fundamentally. When there was no snap indicator, the only information the user had was that snapping is enabled. But with the snap indicator present, we notify him when we'll snap. So why do we sometimes snap without notifying him?
We don't. It's there within 150 msec.
Not quite. The whole point is to "stick" only to the target we snapped to, and ignore other snap targets. To snap to another target, the user has to move the pointer out of range and wait for the snap delay to expire again.
That won't work. If you have a cloud of points, it will not be possible to snap to points in the middle of that cloud. When moving towards that cloud, Inkscape will snap to a point at the circumference and stick there. Only by moving away from the cloud the user can snap again, but by approaching that cloud from a different direction another point on the circumference will be snapped too, and again not the point at the center of the cloud. This can only be solved by taking into account each small motion increment, looking at all points in the vicinity for each motion event, and each time determining which snap target is the closest.
To get the behavior I suggested, I only need to either: a) check whether the distance from the snapped point would be larger than the snap distance, if I snapped to a point. b) compute the nearest point for the path fragment, and check whether it's farther away from the new position than the snap distance, if I snapped to a path. Those computations are trivial and cannot impact performance in any significant way. There's already some processing done on each motion event. Adding this would be insignificant.
See my reasoning above about the cloud of points. Snapping code must be executed on each motion event and your algorithm won't work in this case. The only way to speed this up is to use the snap delay mechanism which eliminates snapping on some of the motion events, to implement better caching of the snap target candidates and their snap points, and to make it faster to find the items in the vicinity (e.g. using some 2D database).
Diederik
Regards, Krzysztof
W dniu 12 stycznia 2010 08:22 użytkownik Diederik van Lierop <mail@...1689...> napisał:
Also note that the suggested new behavior is also trivially predictable: only snap when a snap indicator is present.
No, because the snap indicator can be overlooked or can be disabled. And if that happens the predictability is gone.
What does it mean that it can be "overlooked"? Selective blindness? :) For now let's only consider the case when the snap indicator is enabled, because as I said before the correct behavior without the snap indicator might be different.
The first time I encountered it, it was like this:
- I move the node near another node, with snapping enabled.
- I release the mouse button, and there is no snap indicator. I think
it should stay where I drop it. 3. Bang! It moves to the other node's position. What happen?!? There was no snap indicator, so I expected the node to stay where it is.
Reduce the snap delay and this will be solved.
The behavior at longer snap delays will remain unexpected. Users shouldn't need to tweak settings to fix usability quirks, there should be no usability quirks in the first place.
See my reasoning above about the cloud of points. Snapping code must be executed on each motion event and your algorithm won't work in this case. The only way to speed this up is to use the snap delay mechanism which eliminates snapping on some of the motion events
I think I didn't present the behavior I want clearly enough. This is not an alternative to the snap delay. This feature (let's call it "sticky snap") is intended to complement snap delay, not replace it. In fact it would be very annoying without snap delay. I'll explain "sticky snap" again:
1. I move a node near a snapping point. 2. The snap delay expires, and it snaps. A snap indicator is displayed. 3. As long as the pointer stays within the snap distance from the snaptarget, the node remains snapped to the same place. 4. I move the pointer farther than the snapping distance from the snaptarget. The snap is released, and I can move the node freely until the snap delay expires again.
However, sticky snap is not related to snap delay, and whether we snap on mouse button release. I just think it would make snapping more convenient.
Regards, Krzysztof
On 01/12/2010 02:32 PM, Krzysztof Kosiński wrote:
No, because the snap indicator can be overlooked or can be disabled. And if that happens the predictability is gone.
What does it mean that it can be "overlooked"? Selective blindness? :)
No, it means sitting to close to an 24" monitor ;-). Snapping doesn't always happen close to the mousepointer, so it's possible to miss a few blinking pixels at the other side of the screen!
But anyway, that doesn't really matter.
The behavior at longer snap delays will remain unexpected. Users shouldn't need to tweak settings to fix usability quirks, there should be no usability quirks in the first place.
Please remmber that what works for you might not work for others, and vice versa. I don't feel this is a quirk at all.
But about the usefulness of the snap delay slider you might be right. It should however at least be switchable between 0 and some fixed small delay.
I'll explain "sticky snap" again:
You're explanation is quite clear to me I believe, but I still don't see how you would solve the "cloud" issue when using the sticky snap feature you're proposing. I'll describe the problem in a bit more detail: suppose you have three snap targets close to each other, e.g. within 5 screen pixels distance whereas the snap distance is set to 20 screen pixels. All three snap targets lie on a single vertical line, so when approaching the snap targets from above Inkscape will snap to the upper target, and stick to it. When approaching from below Inkscape will snap to the lower target, and stick to it. But what if you want to snap to the middle point?
PS: The answer I'm looking for is not to approach from the right or left, because I can easily extend this 1D problem to a 2D problem ;-)
Regards,
Diederik
W dniu 12 stycznia 2010 21:50 użytkownik Diederik van Lierop <mail@...1689...> napisał:
What does it mean that it can be "overlooked"? Selective blindness? :)
No, it means sitting to close to an 24" monitor ;-). Snapping doesn't always happen close to the mousepointer, so it's possible to miss a few blinking pixels at the other side of the screen!
I think that people are generally looking at the thing they're dragging, and not on some random places on the screen, so it would be hard to overlook it in practice. The only problematic case is dragging a very large object with the "snap closest only" option disabled. But then the snapping point could well be off the screen.
You're explanation is quite clear to me I believe, but I still don't see how you would solve the "cloud" issue when using the sticky snap feature you're proposing. I'll describe the problem in a bit more detail: suppose you have three snap targets close to each other, e.g. within 5 screen pixels distance whereas the snap distance is set to 20 screen pixels. All three snap targets lie on a single vertical line, so when approaching the snap targets from above Inkscape will snap to the upper target, and stick to it. When approaching from below Inkscape will snap to the lower target, and stick to it. But what if you want to snap to the middle point?
You move the node over the middle point, and wait until the snap delay expires. It snaps to the middle, because this one is closest. There is no snapping before the snap delay, in this respect it works as previously, so you can easily drag the node over one of the snapping points without snapping to it. The only difference is in what happens after the snap.
Regards, Krzysztof
On 01/12/2010 10:10 PM, Krzysztof Kosiński wrote:
You're explanation is quite clear to me I believe, but I still don't see how you would solve the "cloud" issue when using the sticky snap feature you're proposing. I'll describe the problem in a bit more detail: suppose you have three snap targets close to each other, e.g. within 5 screen pixels distance whereas the snap distance is set to 20 screen pixels. All three snap targets lie on a single vertical line, so when approaching the snap targets from above Inkscape will snap to the upper target, and stick to it. When approaching from below Inkscape will snap to the lower target, and stick to it. But what if you want to snap to the middle point?
You move the node over the middle point, and wait until the snap delay expires. It snaps to the middle, because this one is closest.
I expected that, but that will prove to be very difficult when the snap targets are close to each other. It's like darts, aiming for the bull's eye!
Our current implementation doesn't have this problem because it will evaluate all snap targets in the vicinity of the mouse pointer, so a small movement will allow to select another nearby snap target. If we would implement your sticky snap feature then the user would have to move far enough away from where (s)he wants to be and then return to aim for the bull's eye all over again. And aiming will have to be done at a speed above a certain speed threshold because otherwise snapping will occur before arriving at the snap target. This is not acceptable because it's very annoying. It takes control away.
Regards,
Diederik
W dniu 12 stycznia 2010 22:33 użytkownik Diederik van Lierop <mail@...1689...> napisał:
I expected that, but that will prove to be very difficult when the snap targets are close to each other. It's like darts, aiming for the bull's eye!
Our current implementation doesn't have this problem because it will evaluate all snap targets in the vicinity of the mouse pointer, so a small movement will allow to select another nearby snap target. If we would implement your sticky snap feature then the user would have to move far enough away from where (s)he wants to be and then return to aim for the bull's eye all over again. And aiming will have to be done at a speed above a certain speed threshold because otherwise snapping will occur before arriving at the snap target. This is not acceptable because it's very annoying. It takes control away.
Good argument. It can indeed make things harder in this case, if the snap delay is too low to get a precise aim.
If the snap delay is sufficiently high, aiming should not be a big problem. Most people will probably use zooming to disambiguate between close nodes.
On the other hand, sticky snap would make snapping a node to a path easier: I won't have to wait the snap delay after every adjustment to see where my node snaps to the path.
Regards, Krzysztof
On 10/1/10 03:06, Krzysztof Kosiński wrote:
Today I have implemented snapping of nodes, so the new node tool should be ready to go into trunk.
BZR Revision 8976 fails to build on OS X 10.5.8:
CXX ui/tool/node.o ./ui/tool/control-point.h:118: warning: unused parameter ‘state’ ./ui/tool/control-point.h:119: warning: unused parameter ‘event’ ui/tool/node.cpp:306: warning: unused parameter ‘event’ ui/tool/node.cpp:970: warning: unused parameter ‘event’ /usr/include/c++/4.0.0/tr1/hashtable: In copy constructor ‘std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::hashtable(const std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>&) [with Key = Inkscape::UI::SelectableControlPoint*, Value = Inkscape::UI::SelectableControlPoint*, Allocator = std::allocatorInkscape::UI::SelectableControlPoint*, ExtractKey = Internal::identityInkscape::UI::SelectableControlPoint*, Equal = std::equal_toInkscape::UI::SelectableControlPoint*, H1 = std::tr1::hashInkscape::UI::SelectableControlPoint*, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true]’: /usr/include/c++/4.0.0/tr1/unordered_set:56: instantiated from here /usr/include/c++/4.0.0/tr1/hashtable:1045: error: no matching function for call to ‘std::tr1::hashtable<Inkscape::UI::SelectableControlPoint*, Inkscape::UI::SelectableControlPoint*, std::allocatorInkscape::UI::SelectableControlPoint*, Internal::identityInkscape::UI::SelectableControlPoint*, std::equal_toInkscape::UI::SelectableControlPoint*, std::tr1::hashInkscape::UI::SelectableControlPoint*, Internal::mod_range_hashing, Internal::default_ranged_hash, Internal::prime_rehash_policy, false, false, true>::m_allocate_node(Internal::hash_node<Inkscape::UI::SelectableControlPoint*, false>*&)’ /usr/include/c++/4.0.0/tr1/hashtable:898: note: candidates are: typename std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::node* std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::m_allocate_node(const Value&) [with Key = Inkscape::UI::SelectableControlPoint*, Value = Inkscape::UI::SelectableControlPoint*, Allocator = std::allocatorInkscape::UI::SelectableControlPoint*, ExtractKey = Internal::identityInkscape::UI::SelectableControlPoint*, Equal = std::equal_toInkscape::UI::SelectableControlPoint*, H1 = std::tr1::hashInkscape::UI::SelectableControlPoint*, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true] /usr/include/c++/4.0.0/tr1/hashtable:1046: error: request for member ‘copy_code_from’ in ‘* tail’, which is of non-class type ‘Internal::hash_node<Inkscape::UI::SelectableControlPoint*, false>*’ make[2]: *** [ui/tool/node.o] Error 1 make[1]: *** [all-recursive] Error 1 make: *** [all] Error 2 LeWitt:inkscape-bzr suv$
Any further information I can provide?
compiler version:
$ g++ --version i686-apple-darwin9-g++-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) Copyright (C) 2005 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
build configuration:
Configuration:
Source code location: . Destination path prefix: /Volumes/blue/src/Inkscape/src/inkscape-bzr/packaging/macosx/../../Build Compiler: ccache g++ CPPFLAGS: -Wall -Wformat -Wformat-security -W -D_FORTIFY_SOURCE=2 -I/Volumes/blue/mp/include CXXFLAGS: -Wpointer-arith -Wcast-align -Wsign-compare -Woverloaded-virtual -Wswitch -Wno-unused-parameter -O3 -Wall -Wno-strict-aliasing CFLAGS: -Wno-pointer-sign -O3 -Wall LDFLAGS: -L/Volumes/blue/mp/lib Use Xft font database: yes Use gnome-vfs: yes Use openoffice files: yes Use MMX optimizations: no Use relocation support: no Internal Python: skipped Internal Perl: skipped Enable LittleCms: yes Enable Poppler-Cairo: yes ImageMagick Magick++: yes Libwpg: yes
~suv
W dniu 14 stycznia 2010 18:10 użytkownik ~suv <suv-sf@...58...> napisał:
BZR Revision 8976 fails to build on OS X 10.5.8:
At first glance it looks like a bug in <tr1/unordered_set> in your compiler version.
Can you compile the example from the third answer here, to see whether unordered_set works at all? http://stackoverflow.com/questions/1335739/how-to-determine-if-a-list-is-sub...
Regards, Krzysztof
On 14/1/10 19:53, Krzysztof Kosiński wrote:
W dniu 14 stycznia 2010 18:10 użytkownik ~suv <suv-sf@...58...> napisał:
BZR Revision 8976 fails to build on OS X 10.5.8:
At first glance it looks like a bug in <tr1/unordered_set> in your compiler version.
Can you compile the example from the third answer here, to see whether unordered_set works at all? http://stackoverflow.com/questions/1335739/how-to-determine-if-a-list-is-sub...
using the second example from the 3rd answer (Hash Tables):
LeWitt:lists suv$ ll total 24 -rw-r--r--@ 1 suv staff 547 Jan 14 19:57 bit-sets.cpp -rw-r--r--@ 1 suv staff 808 Jan 14 19:56 hash-tables.cpp -rw-r--r--@ 1 suv staff 543 Jan 14 19:57 sorted-lists.cpp LeWitt:lists suv$ make -n hash-tables g++ hash-tables.cpp -o hash-tables LeWitt:lists suv$ make hash-tables
g++ hash-tables.cpp -o hash-tables /usr/include/c++/4.0.0/tr1/hashtable: In copy constructor ‘std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::hashtable(const std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>&) [with Key = int, Value = int, Allocator = std::allocator<int>, ExtractKey = Internal::identity<int>, Equal = std::equal_to<int>, H1 = std::tr1::hash<int>, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true]’: /usr/include/c++/4.0.0/tr1/unordered_set:56: instantiated from here /usr/include/c++/4.0.0/tr1/hashtable:1045: error: invalid conversion from ‘Internal::hash_node<int, false>*’ to ‘int’ /usr/include/c++/4.0.0/tr1/hashtable:1045: error: initializing argument 1 of ‘typename std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::node* std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::m_allocate_node(const Value&) [with Key = int, Value = int, Allocator = std::allocator<int>, ExtractKey = Internal::identity<int>, Equal = std::equal_to<int>, H1 = std::tr1::hash<int>, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true]’ /usr/include/c++/4.0.0/tr1/unordered_set:56: instantiated from here /usr/include/c++/4.0.0/tr1/hashtable:1046: error: request for member ‘copy_code_from’ in ‘* tail’, which is of non-class type ‘Internal::hash_node<int, false>*’ /usr/include/c++/4.0.0/tr1/hashtable: In member function ‘typename std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::const_iterator std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::find(const Key&) const [with Key = int, Value = int, Allocator = std::allocator<int>, ExtractKey = Internal::identity<int>, Equal = std::equal_to<int>, H1 = std::tr1::hash<int>, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true]’: hash-tables.cpp:29: instantiated from here /usr/include/c++/4.0.0/tr1/hashtable:1135: error: passing ‘const std::tr1::hashtable<int, int, std::allocator<int>, Internal::identity<int>, std::equal_to<int>, std::tr1::hash<int>, Internal::mod_range_hashing, Internal::default_ranged_hash, Internal::prime_rehash_policy, false, false, true>’ as ‘this’ argument of ‘typename std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::node* std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::find_node(Internal::hash_node<Value, cache_hash_code>*, const Key&, typename std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::hash_code_t) [with Key = int, Value = int, Allocator = std::allocator<int>, ExtractKey = Internal::identity<int>, Equal = std::equal_to<int>, H1 = std::tr1::h
ash<int>, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true]’ discards qualifiers
make: *** [hash-tables] Error 1 LeWitt:lists suv$
Please bear in mind I don't know anything about C++ myself, nor do I know if I would need any special flags to build that example.
~suv
#include <tr1/unordered_set>
std::tr1::unordered_set<int> create_S() { std::tr1::unordered_set<int> S; S.insert(3); S.insert(2); S.insert(4); S.insert(1); return S; }
std::tr1::unordered_set<int> create_T() { std::tr1::unordered_set<int> T; T.insert(4); T.insert(3); T.insert(5); return T; }
bool includes(const std::tr1::unordered_set<int>& S, const std::tr1::unordered_set<int>& T) { for (std::tr1::unordered_set<int>::const_iterator iter=T.begin(); iter!=T.end(); ++iter) { if (S.find(*iter)==S.end()) { return false; } } return true; }
int main() { std::tr1::unordered_set<int> S=create_S(); std::tr1::unordered_set<int> T=create_T(); return includes(S,T); }
W dniu 14 stycznia 2010 20:06 użytkownik ~suv <suv-sf@...58...> napisał:
using the second example from the 3rd answer (Hash Tables): (snip)
The example should compile without any special flags or other magic. It is definitely a bug in the <tr1/unordered_set> header. You'll have to upgrade to a version of GCC that has a working header. I don't think I can fix this (short of not using unordered_set at all).
Regards, Krzysztof
On 14/1/10 20:26, Krzysztof Kosiński wrote:
W dniu 14 stycznia 2010 20:06 użytkownik ~suv <suv-sf@...58...> napisał:
using the second example from the 3rd answer (Hash Tables): (snip)
The example should compile without any special flags or other magic. It is definitely a bug in the <tr1/unordered_set> header. You'll have to upgrade to a version of GCC that has a working header. I don't think I can fix this (short of not using unordered_set at all).
I have the most up-to-date version of Xcode (3.1.4) installed. Changing from gcc 4.0.1 to 4.2 still doesn't compile the hash-tables test.
LeWitt:lists suv$ echo $CC $CXX /usr/bin/gcc-4.2 /usr/bin/g++-4.2 LeWitt:lists suv$ make hash-tables /usr/bin/g++-4.2 hash-tables.cpp -o hash-tables /usr/include/c++/4.0.0/tr1/hashtable: In copy constructor ‘std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::hashtable(const std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>&) [with Key = int, Value = int, Allocator = std::allocator<int>, ExtractKey = Internal::identity<int>, Equal = std::equal_to<int>, H1 = std::tr1::hash<int>, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true]’: /usr/include/c++/4.0.0/tr1/unordered_set:56: instantiated from here /usr/include/c++/4.0.0/tr1/hashtable:1045: error: invalid conversion from ‘Internal::hash_node<int, false>*’ to ‘int’ /usr/include/c++/4.0.0/tr1/hashtable:1045: error: initializing argument 1 of ‘typename std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::node* std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::m_allocate_node(const Value&) [with Key = int, Value = int, Allocator = std::allocator<int>, ExtractKey = Internal::identity<int>, Equal = std::equal_to<int>, H1 = std::tr1::hash<int>, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true]’ /usr/include/c++/4.0.0/tr1/unordered_set:56: instantiated from here /usr/include/c++/4.0.0/tr1/hashtable:1046: error: request for member ‘copy_code_from’ in ‘* tail’, which is of non-class type ‘Internal::hash_node<int, false>*’ /usr/include/c++/4.0.0/tr1/hashtable: In member function ‘typename std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::const_iterator std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::find(const Key&) const [with Key = int, Value = int, Allocator = std::allocator<int>, ExtractKey = Internal::identity<int>, Equal = std::equal_to<int>, H1 = std::tr1::hash<int>, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true]’: hash-tables.cpp:29: instantiated from here /usr/include/c++/4.0.0/tr1/hashtable:1135: error: passing ‘const std::tr1::hashtable<int, int, std::allocator<int>, Internal::identity<int>, std::equal_to<int>, std::tr1::hash<int>, Internal::mod_range_hashing, Internal::default_ranged_hash, Internal::prime_rehash_policy, false, false, true>’ as ‘this’ argument of ‘typename std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::node* std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::find_node(Internal::hash_node<Value, cache_hash_code>*, const Key&, typename std::tr1::hashtable<Key, Value, Allocator, ExtractKey, Equal, H1, H2, H, RehashPolicy, cache_hash_code, mutable_iterators, unique_keys>::hash_code_t) [with Key = int, Value = int, Allocator = std::allocator<int>, ExtractKey = Internal::identity<int>, Equal = std::equal_to<int>, H1 = std::tr1::h
ash<int>, H2 = Internal::mod_range_hashing, H = Internal::default_ranged_hash, RehashPolicy = Internal::prime_rehash_policy, bool cache_hash_code = false, bool mutable_iterators = false, bool unique_keys = true]’ discards qualifiers
make: *** [hash-tables] Error 1 LeWitt:lists suv$ ll /usr/include/c++/ total 0 drwxr-xr-x 67 root wheel 2278 Oct 24 01:07 4.0.0/
Still need to confirm whether I need additional configuration other than setting $CC and $CXX to use g++-4.2:
LeWitt:lists suv$ which g++-4.2 /usr/bin/g++-4.2 LeWitt:lists suv$ g++-4.2 --version i686-apple-darwin9-g++-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5577) Copyright (C) 2007 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
I can't yet believe that Inkscape will no longer support building with Apple's current Xcode developer tools on OS X Leopard...
:(
~suv
On Jan 14, 2010, at 12:28 PM, ~suv wrote:
I can't yet believe that Inkscape will no longer support building with Apple's current Xcode developer tools on OS X Leopard...
:(
We can't let that happen.
That is, we can't just jump on the latest and greatest compiler, libs, etc. at the expense of common platforms.
At a minimum we should definitely not require anything beyond the deployed OS X versions, and LTS versions of the base Linux distros.
W dniu 14 stycznia 2010 21:28 użytkownik ~suv <suv-sf@...58...> napisał:
I have the most up-to-date version of Xcode (3.1.4) installed. Changing from gcc 4.0.1 to 4.2 still doesn't compile the hash-tables test.
The error messages say you are still using the same broken headers from GCC 4.0.0. Did you also upgrade libstdc++?
I guess the old __gnu_cxx::hash_set should still work. I replaced std::tr1::unordered_set with that, for now. It triggers deprecation warnings in GCC 4.4 though.
Regards, Krzysztof
On 14/1/10 23:44, Krzysztof Kosiński wrote:
W dniu 14 stycznia 2010 21:28 użytkownik ~suv <suv-sf@...58...> napisał:
I have the most up-to-date version of Xcode (3.1.4) installed. Changing from gcc 4.0.1 to 4.2 still doesn't compile the hash-tables test.
The error messages say you are still using the same broken headers from GCC 4.0.0. Did you also upgrade libstdc++?
I did not upgrade anything tonight (the latest Xcode version is needed to build the dependencies for Inkscape in MacPorts, so I had it already installed). Xcode on Leopard installs both versions of gcc, but uses 4.0.1 as default. I never changed that default and only now tried if using 4.2.1 would make a difference.
There are not separate c++ headers for g++-4.2 for 'tr1' (see attached file with the default paths used by the installed gcc/g++ versions)
I guess the old __gnu_cxx::hash_set should still work. I replaced std::tr1::unordered_set with that, for now. It triggers deprecation warnings in GCC 4.4 though.
revision 8980 built ok on OS X 10.5.8
Thank you for the quick response!
:)
~suv
Last login: Thu Jan 14 22:00:45 on ttys003 FYI: ~/.bashrc sourced. LeWitt:~ suv$ export LANG="en_US.UTF-8" LeWitt:~ suv$ gcc -v -x c -E /dev/null Using built-in specs. Target: i686-apple-darwin9 Configured with: /var/tmp/gcc/gcc-5493~1/src/configure --disable-checking -enable-werror --prefix=/usr --mandir=/share/man --enable-languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg][^.-]*$/s/$/-4.0/ --with-gxx-include-dir=/include/c++/4.0.0 --with-slibdir=/usr/lib --build=i686-apple-darwin9 --with-arch=apple --with-tune=generic --host=i686-apple-darwin9 --target=i686-apple-darwin9 Thread model: posix gcc version 4.0.1 (Apple Inc. build 5493) /usr/libexec/gcc/i686-apple-darwin9/4.0.1/cc1 -E -quiet -v -D__DYNAMIC__ /dev/null -fPIC -mmacosx-version-min=10.5.8 -mtune=generic -march=apple ignoring nonexistent directory "/usr/local/include" ignoring nonexistent directory "/usr/lib/gcc/i686-apple-darwin9/4.0.1/../../../../i686-apple-darwin9/include" #include "..." search starts here: #include <...> search starts here: /usr/lib/gcc/i686-apple-darwin9/4.0.1/include /usr/include /System/Library/Frameworks (framework directory) /Library/Frameworks (framework directory) End of search list. # 1 "/dev/null" # 1 "<built-in>" # 1 "<command line>" # 1 "/dev/null" LeWitt:~ suv$ g++ -v -x c++ -E /dev/null Using built-in specs. Target: i686-apple-darwin9 Configured with: /var/tmp/gcc/gcc-5493~1/src/configure --disable-checking -enable-werror --prefix=/usr --mandir=/share/man --enable-languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg][^.-]*$/s/$/-4.0/ --with-gxx-include-dir=/include/c++/4.0.0 --with-slibdir=/usr/lib --build=i686-apple-darwin9 --with-arch=apple --with-tune=generic --host=i686-apple-darwin9 --target=i686-apple-darwin9 Thread model: posix gcc version 4.0.1 (Apple Inc. build 5493) /usr/libexec/gcc/i686-apple-darwin9/4.0.1/cc1plus -E -quiet -v -D__DYNAMIC__ /dev/null -fPIC -mmacosx-version-min=10.5.8 -mtune=generic -march=apple -D__private_extern__=extern ignoring nonexistent directory "/usr/local/include" ignoring nonexistent directory "/usr/lib/gcc/i686-apple-darwin9/4.0.1/../../../../i686-apple-darwin9/include" #include "..." search starts here: #include <...> search starts here: /usr/include/c++/4.0.0 /usr/include/c++/4.0.0/i686-apple-darwin9 /usr/include/c++/4.0.0/backward /usr/lib/gcc/i686-apple-darwin9/4.0.1/include /usr/include /System/Library/Frameworks (framework directory) /Library/Frameworks (framework directory) End of search list. # 1 "/dev/null" # 1 "<built-in>" # 1 "<command line>" # 1 "/dev/null" LeWitt:~ suv$ g++-4.2 -v -x c++ -E /dev/null Using built-in specs. Target: i686-apple-darwin9 Configured with: /var/tmp/gcc_42/gcc_42-5577~1/src/configure --disable-checking --enable-werror --prefix=/usr --mandir=/usr/share/man --enable-languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg][^.-]*$/s/$/-4.2/ --with-slibdir=/usr/lib --build=i686-apple-darwin9 --with-gxx-include-dir=/usr/include/c++/4.0.0 --host=i686-apple-darwin9 --target=i686-apple-darwin9 Thread model: posix gcc version 4.2.1 (Apple Inc. build 5577) /usr/libexec/gcc/i686-apple-darwin9/4.2.1/cc1plus -E -quiet -v -D__DYNAMIC__ /dev/null -fPIC -mmacosx-version-min=10.5.8 -mtune=core2 -D__private_extern__=extern ignoring nonexistent directory "/usr/local/include" ignoring nonexistent directory "/usr/lib/gcc/i686-apple-darwin9/4.2.1/../../../../i686-apple-darwin9/include" #include "..." search starts here: #include <...> search starts here: /usr/include/c++/4.0.0 /usr/include/c++/4.0.0/i686-apple-darwin9 /usr/include/c++/4.0.0/backward /usr/lib/gcc/i686-apple-darwin9/4.2.1/include /usr/include /System/Library/Frameworks (framework directory) /Library/Frameworks (framework directory) End of search list. # 1 "/dev/null" # 1 "<built-in>" # 1 "<command-line>" # 1 "/dev/null" LeWitt:~ suv$
On 14/1/10 23:44, Krzysztof Kosiński wrote:
W dniu 14 stycznia 2010 21:28 użytkownik ~suv <suv-sf@...58...> napisał:
I have the most up-to-date version of Xcode (3.1.4) installed. Changing from gcc 4.0.1 to 4.2 still doesn't compile the hash-tables test.
The error messages say you are still using the same broken headers from GCC 4.0.0. Did you also upgrade libstdc++?
Can you confirm it is part of bug 23465 in gcc that has been resolved long time ago? Apparently Apple never completely fixed the broken version installed with Tiger and Leopard:
Bug 23465 - Assignment fails on TR1 unordered containers: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=23465
Similar issues when compiling other applications on OS X Leopard:
[ABySS-Users] Compiling abyss-1.1.0 on OSX 10.5 http://www.bcgsc.ca/pipermail/abyss-users/2009-December/000567.html
CompilingOnMacOSX < FST < TWiki: http://mohri-lt.cs.nyu.edu/twiki/bin/view/FST/CompilingOnMacOSX
I guess the old __gnu_cxx::hash_set should still work. I replaced std::tr1::unordered_set with that, for now. It triggers deprecation warnings in GCC 4.4 though.
I'll follow up if I have more information about either building a (newer) gcc version via MacPorts or possibly using configure flags for OS X 10.5.8. At the moment I am hesitant to patch individual c++ header files of Apple's gcc as described in above links.
Any further advice would be very appreciated!
~suv
W dniu 15 stycznia 2010 08:39 użytkownik ~suv <suv-sf@...58...> napisał:
Can you confirm it is part of bug 23465 in gcc that has been resolved long time ago? Apparently Apple never completely fixed the broken version installed with Tiger and Leopard:
Yes, we are hitting this bug.
By the way, the configure stanza for Apple's gcc-4.2 looks wrong.
Configured with: /var/tmp/gcc_42/gcc_42-5577~1/src/configure --disable-checking --enable-werror --prefix=/usr --mandir=/usr/share/man --enable-languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg][^.-]*$/s/$/-4.2/ --with-slibdir=/usr/lib --build=i686-apple-darwin9 --with-gxx-include-dir=/usr/include/c++/4.0.0 --host=i686-apple-darwin9 --target=i686-apple-darwin9
Note that is says "--with-gxx-include-dir=/include/c++/4.0.0", and headers <= 4.0.2 are broken. It should say something like "--with-gxx-include-dir=/include/c++/4.2.1". I have no idea why they did this.
The fixes described at http://mohri-lt.cs.nyu.edu/twiki/bin/view/FST/CompilingOnMacOSX should work. Check whether you can compile rev 8979 or the test with one of them. If it works, I will change back to using TR1 unordered_set/map and add some configure code to detect the broken headers.
Regards, Krzysztof
participants (7)
-
bulia byak -
Diederik van Lierop -
Jon Cruz -
Joshua A. Andler -
Krzysztof Kosiński -
Nicolas Dufour -
~suv