I have some doubts about the adaptable UI feature.
Part 1: UI issues - The UI mode selection combo box (called "Task" for some reason) has usability problems: 1. it jumps around when changing mode, so every time I change it, I have to find it again to try another mode; 2. when the main commands toolbar is vertical, it causes a very large horizontal padding on it, even when it's hidden in the overflow menu. - Tearing off toolbars does not adjust their size to fit the controls. If I tear off a toolbar from a small window, I get a toolbar with an overflow menu. If I tear off from a maximized window, I get a toolbar with lots of unused space. This makes tearing off highly useless. - Toolbars can be torn off but can't be attached in a location different that the original one. This is because they use GtkHandleBox, for which the docking position is fixed. At the same time GDL has a widget named GdlDockBar which sounds like a true dockable toolbar (but it might as well be something else, I don't know).
Part 2: UxManager class - UxManager::setTask: Magic constants in switch. - UxManager constructor: "tags" is created but doesn't do anything. - What is the point of having several desktops attached to one UxManager?
Part 3: util/ege-tags.h, util/ege-tags.cpp - ege-tags.h: It supposedly implements http://create.freedesktop.org/wiki/ResourceTagging but at present I don't see any place in Inkscape that could make use of tagging. - Even if the aforementioned specification was good, the UxManager class apparently tries to do two completely unrelated things: manage external resources and change the UI mode. - TagSet: if I understand correctly what this class attempts to do, it should use std::tr1::unordered_map<Tag, int> with appropriate hash and equality predicates instead of the combination of std::vector<Tag> and std::map<std::string, int>.
Part 4: other problems - toolbox.cpp:1578: code related to changing the UI layout uses a switch to assign function pointers. Polymorphism should be used instead.
Regards, Krzysztof
On 2/18/10, Krzysztof Kosiński wrote:
I have some doubts about the adaptable UI feature.
Part 1: UI issues
May I chime in? No.1 issue for me is that no matter how hard I look I just can't see where exactly all those combo boxes are (or whatever it is implemented as). I do see relevant messages in toolbar relates source code files when I look at the PO file, but I really can't find this in real UI.
Alexandre
On Feb 17, 2010, at 1:48 PM, Krzysztof Kosiński wrote:
I have some doubts about the adaptable UI feature.
Part 1: UI issues
- The UI mode selection combo box (called "Task" for some reason)
That's actually a very simple and straightforward issue. The naming is from the industry standard for such things. You really should read some of the information that has been presented, mentioned in discussions, linked to, etc. It is also very important to understand the purpose of a feature to be able to asses and to assist with what might be appropriate architecture to implement it.
Since you seem to have missed it a few times, I'll relink again:
Michael Terry's presentation from the Libre Graphics Meeting last year: http://river-valley.tv/ingimp-a-smorgasbord-of-usability-adaptive-uis-and-vi...
The slides from that presentation: http://www.ingimp.org/sites/default/files/michael_terry_lgm_2009.pdf
His overall page: http://hci.uwaterloo.ca/faculty/mterry/
I also gave a presentation on this at linux.conf.au. http://www.lca2010.org.nz/programme/schedule/view_talk/50299?day=friday Once they get the slides and videos posted, I'll link to those too.
Again, it very much helps to drop in to our chat room once in a while. Aside from getting an idea what goes on for different users and other devs, you can ask questions and get general pointers
On Feb 17, 2010, at 1:48 PM, Krzysztof Kosiński wrote:
I have some doubts about the adaptable UI feature.
Part 1: UI issues
- The UI mode selection combo box (called "Task" for some reason) has
usability problems: 1. it jumps around when changing mode, so every time I change it, I have to find it again to try another mode; 2. when the main commands toolbar is vertical, it causes a very large horizontal padding on it, even when it's hidden in the overflow menu.
Yes, the UI is not done yet. Focus was given to implementing the functionality first, with the UI to naturally follow via "form follows function". This has allowed others to try it out and provide guiding feedback, including many from linux.conf.au.
- Tearing off toolbars does not adjust their size to fit the controls.
If I tear off a toolbar from a small window, I get a toolbar with an overflow menu. If I tear off from a maximized window, I get a toolbar with lots of unused space. This makes tearing off highly useless.
No, it does not.
That is the behavior that was researched and settled upon some time ago (I'm surprised you did not raise this issue a few releases ago). Although you personally do not like it, that behavior makes things *very* usable for many people out there. One segment of the userbase that preferred it that way are those with laptops and netbooks.
You mention that you get a toolbar with lots of unused space. That is because you get exactly what you ask for. When torn off, the toolbar keeps the size that it held while docked. If you want larger toolbars just tear them off while the window is larger. If you want smaller toolbars then just tear off when the window is smaller. It leaves the user in complete control with no need to write any additional code.
Among other things this turned out to be a very simple yet usable way to prevent toolbars from becoming to large for the screen. (I know that MS Windows imposes a sytem-wide limit on sizes, but most other OS's do not). If you care to take a pass at creating a better solution, please let me know. I have most of the research from when that was originally done, and can pass on to you all the base requirements (such as that "too large" problem) that something more "automatic" would require.
- Toolbars can be torn off but can't be attached in a location
different that the original one. This is because they use GtkHandleBox, for which the docking position is fixed. At the same time GDL has a widget named GdlDockBar which sounds like a true dockable toolbar (but it might as well be something else, I don't know).
Yes, GDL needs some major refactoring. It was originally forked into our codebase when GDL was heading in a different direction, and was not interested in the bulk of the changes we wanted. It also was targeting a more "kitchen sink" approach than it is taking now.
What needs to happen is
1) determine where the different forks of GDL have gone 2) assess where the prime GDL is at the moment, and what changes would be needed 3) coordinate with other projects that have forked and/or wrapped GDL and plan a unified C++ API. 4) push needed changes upstream to the prime GDL. 5) switch to using the prime GDL.
There is quite a set of different tasks to work on in all this, so it is a perfect area for you (or any others) to lend a hand in. It would be greatly appreciated, and by more than just the Inkscape community.
Part 2: UxManager class
- UxManager::setTask: Magic constants in switch.
Yes. Temporary code to enable working on the feature. Once the functionality comes clear, the needed UI becomes much easier to implement.
- UxManager constructor: "tags" is created but doesn't do anything.
... yet.
The tags code provide needed functionality for the UI, and has been used already for some experimental work (AKA not clean enough for trunk). Additionally there are people in other projects who are interested is sharing that code, so going into the trunk sooner rather than later was a request. It is also the reason the tags files need to stay pure C++ and not use GTK+.
- What is the point of having several desktops attached to one UxManager?
Read the papers on adaptive UI, watch Michael Terry's presentation, etc. Let me know if that still has not become clear once you go over the source material.
Part 3: util/ege-tags.h, util/ege-tags.cpp
- ege-tags.h: It supposedly implements
http://create.freedesktop.org/wiki/ResourceTagging but at present I don't see any place in Inkscape that could make use of tagging.
See the notes above on "tags".
However, if you are saying that you can not visualize areas of our interface and data resources that could benefit from having tags applied, then that is a very different issue. Is that what you were trying to state?
- Even if the aforementioned specification was good, the UxManager
class apparently tries to do two completely unrelated things: manage external resources and change the UI mode.
No, it does even more than that, *HOWEVER* those are NOT unrelated at all. Once you review the source material on adaptive UI then it will be easier to explain "big picture" architecture.
- TagSet: if I understand correctly what this class attempts to do, it
should use std::tr1::unordered_map<Tag, int> with appropriate hash and equality predicates instead of the combination of std::vector<Tag> and std::map<std::string, int>.
No. Not at all.
One of the prime things we want to do is keep our codebase working for a wide range of end users and distros. We do *NOT* want to require the latest and greatest of each and every library. Thus the unordered_map template is one that needs to be avoided as much as possible. My prime concern there at the moment is that it is a new thing that is just going in and is not yet 'standard' (behavior, implementation, use, etc.) on even the newest few platforms, let alone the range we would like to keep covered.
But I do want to take care and emphasize that the immediate problem with unordered_map is that its use is buggy and prone to implementation issues on the main few platforms we initially target. Use of buggy/problematic types/code should be minimized.
And more important to this specific case is that you need to keep in mind that projects other than Inkscape are wanting to share that code. Keeping it clean and more portable thus gains in importance.
Part 4: other problems
- toolbox.cpp:1578: code related to changing the UI layout uses a
switch to assign function pointers. Polymorphism should be used instead.
No, it should not.
If you review the overall use of things there (ignoring the temporary use of int's to pin tasksets) you'll see that the int used in the switch is coming in through a public enum value on the public API (or the rough equivalent thereof). Polymorphism really won't contribute anything at this point in time, and most likely will add significant complexity. Complexity is the enemy, and should be minimized whenever practical.
2010/2/20 Jon Cruz <jon@...18...>:
Yes, the UI is not done yet. Focus was given to implementing the functionality first, with the UI to naturally follow via "form follows function". This has allowed others to try it out and provide guiding feedback, including many from linux.conf.au.
If the UI is incomplete, why is it in trunk? Wouldn't it be better to keep it in a branch until it's ready for general use?
The presentations you mentioned are not about moving toolbars around, they are about showing different sets of commands depending on the chosen task. When the task changes, the toolbars should not move around, so that commands common to several tasks stay in the same places.
- determine where the different forks of GDL have gone
- assess where the prime GDL is at the moment, and what changes would be needed
- coordinate with other projects that have forked and/or wrapped GDL and plan a unified C++ API.
- push needed changes upstream to the prime GDL.
- switch to using the prime GDL.
I looked at this some time ago, the only changes in our tree are: 1. Some fixes that probably deal with floating dialogs 2. Ability to use a pixmap images as the dockbar icon (original GDL requires stock items) In fact we might be able to compile Inkscape with stock GDL if we create stock items from our icons on the fly.
However, if you are saying that you can not visualize areas of our interface and data resources that could benefit from having tags applied, then that is a very different issue. Is that what you were trying to state?
I see what resources they can be applied to (dash patterns, markers, patterns, gradients, etc.) but the UI for operating on them needs significant changes in order to use tags in any way.
- Even if the aforementioned specification was good, the UxManager
class apparently tries to do two completely unrelated things: manage external resources and change the UI mode.
No, it does even more than that, *HOWEVER* those are NOT unrelated at all. Once you review the source material on adaptive UI then it will be easier to explain "big picture" architecture.
I read that and the only relation I see is showing specifically tagged resources when a task is selected. UxManager (whatever that name means) should not manage all resources - it should only expose a method to get the current resource filter which could then be applied to resource selectors (e.g. the gradient selector).
One of the prime things we want to do is keep our codebase working for a wide range of end users and distros. We do *NOT* want to require the latest and greatest of each and every library.
unordered_set / unordered_map are not the latest and greatest. They are available since GCC 4.0.0 which was released 5 years ago. By comparison GTK 2.10, which is the oldest version that has a chance of working with Inkscape, was released 4 years ago. It works absolutely everywhere, including MSVC and Intel C++, except the misconfigured Apple GCC. (Note that it's their fault, because they configured the compiler to use a non-matching set of standard C++ headers - their g++ 4.2.1 uses headers from g++ 4.0.0. If they didn't do that, it would work correctly.)
the immediate problem with unordered_map is that its use is buggy and prone to implementation issues
You are just making stuff up to excuse Apple from providing a broken toolchain. unordered_map is no more "buggy" or "prone to implementation issues" than std::vector or std::string.
And more important to this specific case is that you need to keep in mind that projects other than Inkscape are wanting to share that code. Keeping it clean and more portable thus gains in importance.
This is another reason to use unordered_set/map. They are more portable than the GCC-specific hash implementations we used before. For example they also work with MSVC and Intel C++, whereas __gnu_cxx::hash_set/map doesn't.
Regards, Krzysztof
On Feb 23, 2010, at 1:44 PM, Krzysztof Kosiński wrote:
2010/2/20 Jon Cruz <jon@...18...>:
Yes, the UI is not done yet. Focus was given to implementing the functionality first, with the UI to naturally follow via "form follows function". This has allowed others to try it out and provide guiding feedback, including many from linux.conf.au.
If the UI is incomplete, why is it in trunk? Wouldn't it be better to keep it in a branch until it's ready for general use?
Well, first of all the *feature* is ready for some general testing, even though the UI is not yet polished. We discussed this in the chat room, and many people asked me to put it in. In fact, I even had people come up to me at linux.conf.au the few days after I checked in and ask for a "wishlist" that turned out to be in the code already.
Second, this is new functionality, and does not break nor change old functionality. The worst case is that some of the items are taking a bit much horizontal space (well... that one toolbar takes waaaaaay too much)... however that only becomes visible if someone is trying the new functionality. That's been our policy since the beginning. Potentially risky changes or changes to existing behavior should be done on a branch, but otherwise things should go in trunk to gain exposure and avoid bit-rot.
(branches and #ifdef both count as high potential for bit-rot. Avoiding that, and especially the #ifdef case, had been one of Mental's well justified pet peeves.)
The presentations you mentioned are not about moving toolbars around, they are about showing different sets of commands depending on the chosen task. When the task changes, the toolbars should not move around, so that commands common to several tasks stay in the same places.
No, not really.
Perhaps it was that they focused first on sub-setting the functionality, and did not *initially* address moving of items.
It is true that keeping things consistent is good and has it's place, but on the other hand the key is to focus on *tasks* and not on *commands*. In several tasks, keeping a given command in a certain spot will most likely make sense, but in *other* tasks it might make sense for some of them to move.
There were some interesting things in those presentations, but some might have been quite subtle. I can assure you, though, that I spent quite a lot of time with Michael Terry and his team. We were going over all sorts of issues (did you happen to catch the one on movement templates?) and I first asked a lot of questions to be sure I had the correct understanding of what was presented.
- determine where the different forks of GDL have gone
- assess where the prime GDL is at the moment, and what changes would be needed
- coordinate with other projects that have forked and/or wrapped GDL and plan a unified C++ API.
- push needed changes upstream to the prime GDL.
- switch to using the prime GDL.
I looked at this some time ago, the only changes in our tree are:
- Some fixes that probably deal with floating dialogs
- Ability to use a pixmap images as the dockbar icon (original GDL
requires stock items) In fact we might be able to compile Inkscape with stock GDL if we create stock items from our icons on the fly.
That might be true for the current behavior. However we really need to *improve* the behavior. This has also been discussed some with the current GDL team, and other projects who themselves have branched or re-branched GDL.
However, if you are saying that you can not visualize areas of our interface and data resources that could benefit from having tags applied, then that is a very different issue. Is that what you were trying to state?
I see what resources they can be applied to (dash patterns, markers, patterns, gradients, etc.) but the UI for operating on them needs significant changes in order to use tags in any way.
Oh, no. Not really.
There are many subtle things about our UI that actually make it not nearly as much work as you might think. For example, the code that runs the set of swatches shown at the bottom and side of the window is generic code that was designed to show any type and not just colors. In fact, the code is already in there that has gradients also showing up.
So, first adding a "jump to tagged item" functionality is probably a day or so's work.
Filtering the view to only matching tags is probably not that much work either. Things are based on "previewables" that are objects that can be previewed. Add "getTags()" to that interface and then the PreviewHolder (I think that's the class) can be tweaked to filter the view. It is already presenting the data set in a dynamic and rearranging manner.
- Even if the aforementioned specification was good, the UxManager
class apparently tries to do two completely unrelated things: manage external resources and change the UI mode.
No, it does even more than that, *HOWEVER* those are NOT unrelated at all. Once you review the source material on adaptive UI then it will be easier to explain "big picture" architecture.
I read that and the only relation I see is showing specifically tagged resources when a task is selected. UxManager (whatever that name means) should not manage all resources - it should only expose a method to get the current resource filter which could then be applied to resource selectors (e.g. the gradient selector).
No, that is not the design intent of UxManager.
UxManager is the "User experience manager" ("UX" is the current industry term that is related to things like "user interation", "human interface design", etc). It should manage the user experience overall. This is a fairly high-level component and will run several things.
One such job is to control the appearance of windows and sub-windows. It will also need to know context of the current work so it can properly adjust in real-time. Knowing where different windows are is very important here.
It can move windows around. It can dock or float tool windows. It can bring up duplicate windows with special views (icon preview, animation timeline, rsvg preview, etc.).
It can see that you are running on a netbook and default to have toolbars on the side. It can remember that last time you moved the toolbars back to the top, so this time it will leave them where you had them, etc.
Basically the UxManage is supposed to be the AI that will run things and help you out as you work.
... Therefore the UxManager needs to be a single instance that is aware of more than just a single window on a document. If it were per-desktop then even a single document would end up with several.
One of the prime things we want to do is keep our codebase working for a wide range of end users and distros. We do *NOT* want to require the latest and greatest of each and every library.
unordered_set / unordered_map are not the latest and greatest. They are available since GCC 4.0.0 which was released 5 years ago. By comparison GTK 2.10, which is the oldest version that has a chance of working with Inkscape, was released 4 years ago. It works absolutely everywhere, including MSVC and Intel C++, except the misconfigured Apple GCC. (Note that it's their fault, because they configured the compiler to use a non-matching set of standard C++ headers - their g++ 4.2.1 uses headers from g++ 4.0.0. If they didn't do that, it would work correctly.)
Just look through our autoconf setup. There are many such issues. And hash_set has been around for much longer, but it is just as optional as unordered_map. (remember, the tr1 items are just a suggestion).
the immediate problem with unordered_map is that its use is buggy and prone to implementation issues
You are just making stuff up to excuse Apple from providing a broken toolchain. unordered_map is no more "buggy" or "prone to implementation issues" than std::vector or std::string.
No, I'm not.
And, yes, std::vector and std::string *ALSO* used to have problems. I've shipped software in those days too, and a little config magic made it works beautifully then too. Even system calls such as strstr() might be a problem on certain platforms (ask me about Sun some time if you are wondering).
Now, I do know that on the majority of current Linux distributions tr1/unordered_map is working well. I do not dispute that. But then on other systems, especially with compilers other than gcc, things are not as stable. (did you know Netware is still a supported OS with supported third party software?)
And more important to this specific case is that you need to keep in mind that projects other than Inkscape are wanting to share that code. Keeping it clean and more portable thus gains in importance.
This is another reason to use unordered_set/map. They are more portable than the GCC-specific hash implementations we used before. For example they also work with MSVC and Intel C++, whereas __gnu_cxx::hash_set/map doesn't.
A bit... but then again just a tiny bit of autoconf work will make our code use whatever is best for the platform it is compiled on.
But... many times (please note this, as you seemed to have missed this before) using those might not be the best choice. I'm not saying I know that this is one of those cases, but that it *might* be so we should take just a little time to check (I'm trying to stress that we just need to check, and that I'm not sure of things one way or the other yet). Very often instead of trying to make certain operations faster it is better to actually just not do those operations and take a different approach instead.
I'm seeing the potential here of the type of optimization Michael Abrush wrote about where the code optimizations might get to improve performance by a factor of 10 whereas the high-level change of approach can get 100 times the performance. (He also stressed actually measuring to be sure the theoretical actually showed up in the real program).
W dniu 24 lutego 2010 11:59 użytkownik Jon Cruz <jon@...18...> napisał:
It is true that keeping things consistent is good and has it's place, but on the other hand the key is to focus on *tasks* and not on *commands*. In several tasks, keeping a given command in a certain spot will most likely make sense, but in *other* tasks it might make sense for some of them to move.
Yes, certainly; but the current choices in the combo box - "Widescreen" and "Custom" - are not tasks. They are toolbar layouts. It would be better if the task checkbox changed the available commands, while the toolbars could be dragged and docked to different edges of the window as the user wants and stay there.
UxManager is the "User experience manager" ("UX" is the current industry term that is related to things like "user interation", "human interface design", etc). It should manage the user experience overall. This is a fairly high-level component and will run several things.
"User experience" is not a concept that can have its dedicated object - every line of code in the program affects it. Similarly you can't have meaningful "UsabilityManager" or "InkscapePopularityBooster" classes. What does "managing the user experience" actually mean? I am afraid that this class will quickly become a bag of unrelated things or a God object. There should be a better name for it, for example "ToolbarLayoutManager", "TaskManager", "CommandSet" or "TagStorage".
(BTW, we have an awful lot of "managers", sounds like an invasion of suits :) )
One such job is to control the appearance of windows and sub-windows. It will also need to know context of the current work so it can properly adjust in real-time. Knowing where different windows are is very important here. It can move windows around. It can dock or float tool windows. It can bring up duplicate windows with special views (icon preview, animation timeline, rsvg preview, etc.).
For icon preview and RSVG preview, I see no reason to have an extra class to create them - just create a preview dialog in response to action activation like any other. Docking would be better left to GDL and the user (unless you have some specific examples where something more complex needs to be done). Moving windows is a bad idea - the window manager should do it.
It can see that you are running on a netbook and default to have toolbars on the side. It can remember that last time you moved the toolbars back to the top, so this time it will leave them where you had them, etc.
For user setting persistence we have preferences - it's better to add whatever functionality you need to them. The logic to determine where the toolbars should be by default based on screen resolution can be put somewhere in the toolbar creation code.
Basically the UxManage is supposed to be the AI that will run things and help you out as you work.
Can you provide more specific examples of "smart" behavior which would be triggered by this class? We should avoid trying to be smarter than the user, because it is usually very annoying.
Now, I do know that on the majority of current Linux distributions tr1/unordered_map is working well. I do not dispute that. But then on other systems, especially with compilers other than gcc, things are not as stable. (did you know Netware is still a supported OS with supported third party software?)
Every system that has (a correctly configured) GCC 4.0.2 or later will work. However, I just noticed we can circumvent this problem by using the Boost versions of unordered containers. I recall that they didn't want to work for some reason when I first tried them, but I'll try again.
Regards, Krzysztof
2010/2/24 Krzysztof Kosiński <tweenk.pl@...400...>:
"User experience" is not a concept that can have its dedicated object
- every line of code in the program affects it. Similarly you can't
have meaningful "UsabilityManager" or "InkscapePopularityBooster" classes. What does "managing the user experience" actually mean? I am afraid that this class will quickly become a bag of unrelated things or a God object. There should be a better name for it, for example "ToolbarLayoutManager", "TaskManager", "CommandSet" or "TagStorage".
I agree with Krzysztof here. Jon, to me "user experience" is an almost meaningless marketing phrase. Could you try and find something better?
By the way your "eek" files, still in the codebase, are not exactly a good naming example either, even if you invent a backronym for them :)
For icon preview and RSVG preview, I see no reason to have an extra class to create them - just create a preview dialog in response to action activation like any other. Docking would be better left to GDL and the user (unless you have some specific examples where something more complex needs to be done). Moving windows is a bad idea - the window manager should do it.
We have something similar for normal and fullscreen modes: the set of visible toolbars and other elements is different and independently remembered in these modes. In any case, this code should then be combined with the new "ux" code.
For user setting persistence we have preferences - it's better to add whatever functionality you need to them. The logic to determine where the toolbars should be by default based on screen resolution can be put somewhere in the toolbar creation code.
Definitely, all UI for this feature, whatever it is, should be in Preferences.
On Mar 2, 2010, at 7:49 AM, bulia byak wrote:
2010/2/24 Krzysztof Kosiński <tweenk.pl@...400...>:
"User experience" is not a concept that can have its dedicated object
- every line of code in the program affects it. Similarly you can't
have meaningful "UsabilityManager" or "InkscapePopularityBooster" classes. What does "managing the user experience" actually mean? I am afraid that this class will quickly become a bag of unrelated things or a God object. There should be a better name for it, for example "ToolbarLayoutManager", "TaskManager", "CommandSet" or "TagStorage".
I agree with Krzysztof here. Jon, to me "user experience" is an almost meaningless marketing phrase. Could you try and find something better?
Well, it's not a marketing phrase.
It's now the industry standard (human interface design, HCI, HIG, etc, industry) for what is being addressed.
http://en.wikipedia.org/wiki/User_experience
So any people who work or even look into usability are using that. Most people working in usability are addressing "UX". The term has moved in to the mainstream on that front. Most people I've talked with at conferences, etc, for the last two years now has been speaking of "usability" and "UX".
By the way your "eek" files, still in the codebase, are not exactly a good naming example either, even if you invent a backronym for them :)
Yes. Though I did have an acronym for them from the beginning. Once we open things up again I'll go through and get those renamed with "ege".
For icon preview and RSVG preview, I see no reason to have an extra class to create them - just create a preview dialog in response to action activation like any other. Docking would be better left to GDL and the user (unless you have some specific examples where something more complex needs to be done). Moving windows is a bad idea - the window manager should do it.
We have something similar for normal and fullscreen modes: the set of visible toolbars and other elements is different and independently remembered in these modes. In any case, this code should then be combined with the new "ux" code.
Yes, exactly. As soon as I have a break in coding I was going to explain to Krzysztof about "*Manager" being the common naming for a facade pattern, and how employing such design patterns can be helpful.
The very tricky part here is to layer things up properly so we can gain the most. We very much need to get "business logic" out of the toolbar code and change from so much of the hardcoded support we have now (where even some widgets getting displayed initialize internals that otherwise are left random) and get a proper distribution of responsibility.
To address the specific there, the "fullscreen" boolean is just one aspect in the current UX set. Those will need to be combined and managed in a data-driven manner. Otherwise the combination of settings will explode.
For user setting persistence we have preferences - it's better to add whatever functionality you need to them. The logic to determine where the toolbars should be by default based on screen resolution can be put somewhere in the toolbar creation code.
Definitely, all UI for this feature, whatever it is, should be in Preferences.
Somewhat...
The preference mechanism is good for storing sets of data, but hooking it in to the low-level of each widget construction function is not how we should implement it. Basically Krzysztof seems to be thinking here of here of spreading the business logic throughout all vertical layers, whereas what we need is to think of things more in an N-tier architecture type of approach with the UI cleanly separated from such decisions.
On Tue, Mar 2, 2010 at 2:24 PM, Jon Cruz <jon@...18...> wrote:
Well, it's not a marketing phrase.
OK, it's a phrase from usability research. It doesn't make it any better for the name of a class.
"UsabilityManager" makes no sense, and neither does "UserExperienceManager".
The preference mechanism is good for storing sets of data, but hooking it in to the low-level of each widget construction function is not how we should implement it.
It's not hooking into low-level. It's the other way round. All levels just use prefs for storing their data. The way my toolbars are positioned is clearly a piece of data I want to store and preserve.
By the way, will we have no chooser of tools for the main toolbox in 0.48? That's really bad, it is quickly becoming clumsy with new tools added.
On Mar 2, 2010, at 10:50 AM, bulia byak wrote:
On Tue, Mar 2, 2010 at 2:24 PM, Jon Cruz <jon@...18...> wrote:
Well, it's not a marketing phrase.
OK, it's a phrase from usability research. It doesn't make it any better for the name of a class.
"UsabilityManager" makes no sense, and neither does "UserExperienceManager".
I think it makes a lot of sense.
It "Manages the user experience".
If a wants to switch to a remembered config, that will run it. If a bit of code needs to know some aspect of preference along a known axis, that code can as the ux manager, etc.
2010/3/3 Jon Cruz <jon@...18...>:
"UsabilityManager" makes no sense, and neither does "UserExperienceManager". I think it makes a lot of sense. It "Manages the user experience".
What does "managing the user experience" actually mean? If you want a class called "UILayoutManager", it's acceptable. But "managing the user experience" has no meaning that can be expressed in code.
Also I think *how* the current style widget is updated has a good example. Instead of "peeking out" via SP_ACTIVE_DOCUMENT() or such, the widget has it's dependency "pushed in" via its setDesktop() call. In theory we could do a simple floating dialog that creates and holds a single current style widget. That floating dialog could listen for the "active" desktop to change and then when it hears an update that dialog can turn around and "push down" into the current style widget by calling setDesktop() on it. That's the kind of approach I'm trying to look at.
The idea of re-attaching the floating dialog to a different window is worrying me. Either the dialog should be assigned to some window and stay assigned to it until it's closed, or the dialog should always be assigned to the currently active window. In the first case the desktop should be a construction parameter of the dialog. In the second case using SP_ACTIVE_DESKTOP or an equivalent together with a change signal is OK. If we store the desktop locally and change it using setDesktop() we are at risk of getting out of sync in some obscure code path. I still don't see how an "user experience manager" class might help here.
It doesn't matter whether you use setDesktop() or a construction parameter - there's still a dependency on SPDesktop, and it cannot be removed.
Regards, Krzysztof
On Thu, 2010-03-04 at 16:17 +0100, Krzysztof Kosiński wrote:
The idea of re-attaching the floating dialog to a different window is worrying me.
I don't use the feature, but wouldn't this be useful with View>Duplicate window? I don't see it in any other workflow, but if people use that duplicate window stuff it would seem reasonable.
Cheers, Josh
W dniu 4 marca 2010 17:36 użytkownik Joshua A. Andler <scislac@...400...> napisał:
I don't use the feature, but wouldn't this be useful with View>Duplicate window? I don't see it in any other workflow, but if people use that duplicate window stuff it would seem reasonable.
In option 1 (floating dialogs always refer to the active window), the active window would change when it's duplicated - nothing new. The dialogs might receive a signal that notifes them of the change of active window. In option 2 (floating dialogs refer to the window that created them), the dialogs should be duplicated. No reattachment takes place.
By "reattaching the floating dialog to a different window", I mean option 2 but with the possibility to change the window to which the dialog refers. I think it creates unnecessary complexity, and option 2 is confusing anyway. Option 1 is better.
There's extra complexity from the fact that dialogs can be either docked or floating. As I said before, having several floating dialogs of the same type that refer to different windows might be very confusing. At the same time it should be possible to have multiple docked dialogs of the same type in different windows that should always refer to the window they're docked in. I think the best place to solve it is the base class of all dialogs.
There are some extra considerations: - I have a floating Fill & Stroke, and I undock another Fill & Stroke. (I would close the old dialog.) - I dock a floating dialog to a different window than it was undocked from. Does GDL allow this? - New dialogs are set to dock by default, and I have a defocused floating Fill & Stroke dialog. I press a key shortcut or menu command to open Fill & Stroke from some window. Is the existing dialog brought to the front, or does a docked version appear? (My choice is that the existing dialog is brought to the front, because it's the "shared" version that in some sense belongs to all windows. However, this could be annoying on Linux with multiple workspaces.)
Regards, Krzysztof
On Mar 4, 2010, at 7:17 AM, Krzysztof Kosiński wrote:
The idea of re-attaching the floating dialog to a different window is worrying me. Either the dialog should be assigned to some window and stay assigned to it until it's closed, or the dialog should always be assigned to the currently active window. In the first case the desktop should be a construction parameter of the dialog. In the second case using SP_ACTIVE_DESKTOP or an equivalent together with a change signal is OK. If we store the desktop locally and change it using setDesktop() we are at risk of getting out of sync in some obscure code path. I still don't see how an "user experience manager" class might help here.
It doesn't matter whether you use setDesktop() or a construction parameter - there's still a dependency on SPDesktop, and it cannot be removed.
Oh, sorry for the delay on this response. (I just found I had it hiding in my "Drafts" folder)
The purpose of a floating dialog, at least the most common reason, is to work on the "current" view. Even if one has a single document open, it can have multiple views. Through most of it's earlier life Inkscape worked this way and only this way.
A docked panel, on the other hand, should operate only on the view it is embedded in. So we should have two states: "when floating, operate on current" and then "when docked, operate only on the owner". But then again, we have only a single state : "switch to work on the desktop as it is assigned to you". Approaching things with that single state mindset actually makes things easier.
The problem we have is from lower-level components being coded to look "up" the hierarchy outside of themselves with hardcoded assumptions about how they will be used. Since they have been hardcoded to be used in one and only one situation, then they can not reliably work in other situations.
With the introduction of the GDL docking code, this shortcoming in design became apparent. The problem is that the hardcoded assumptions the widgets held inside themselves no longer applied, thus they broke in various ways. The worst of which results in crashing (and I think that was addressed by a work-around of never destructing panels and/or dialogs).
Now, it is true that these meta components are dependent on an SPDesktop. However, they do not need to depend on "fetch the desktop pointed to by the global variable held in a known location". Instead it is easy to apply dependency injection (though we probably want to stick with manual instead of framework driven). The *KEY* yet subtle difference here is in changing things away from "a dialog reaching out and grabbing the global pointer desktop" to instead have "a dialog uses the desktop that is handed to it"
http://en.wikipedia.org/wiki/Dependency_Injection
Now, I should probably point out that we are *very* close to having those. That is, with just a bit of refactoring we can reach successful dependency injection on the dialogs and panels.
I am glad that reattaching was worrying you. It properly should. That is, your concerns are most likely well justified in this area. Concern is good because it will help us address this potentially buggy area and get things fixed. While it is true that there is risk in changing things to by dynamically set, there is actually also significant risk in using global variables carelessly.
Specifically I've seen problems in the existing code in that large complex functions will use globals multiple times within the whole of the function instead of storing to a local variable at the beginning and working with that variable. In some of the code I've inspected there is a high risk of parts from different desktops being mixed within the run of a single function. We're not seeing things now since we're very single threaded, but as we move more to use multiple cores we will start to trip over these issues.
And I think one big takeaway here is that global variables are very bad for multithreading. We want multithreading. So we need to reduce and/or eliminate the use of global variables.
On Mar 2, 2010, at 10:50 AM, bulia byak wrote:
The preference mechanism is good for storing sets of data, but hooking it in to the low-level of each widget construction function is not how we should implement it.
It's not hooking into low-level. It's the other way round. All levels just use prefs for storing their data. The way my toolbars are positioned is clearly a piece of data I want to store and preserve.
Yes, yes.
But the key here is *which* bit of code store *which* pieces of information and in *what* format. As they say, the devil is in the details.
So under different circumstances, different settings are pertinent. Either the low-level needs to know the entirety of possible context information, or something higher up needs to run settings instead.
Something low-level like a button might be made to do something the equivalent of:
if (Inkscape::Application::getGlobalApp()::getWindowCount() > 0)
etc. In this case something at the lower levels is looking *up* through the program organization and searching for global information that it has been hardcoded to know something about.
Generally it has been taken that it is better to have low-level components encapsulated and modular. If they do one thing and one thing only, and in a constrained manner, then it is much easier to do things such as reuse a software component in a different way or in a different location.
I think the color selectors might be a quick example from our codebase. They are used several places in our code, and do not read nor write any preferences themselves. Instead something a bit higher in the chain creates them, pushes initial values into them, listens to changes coming from them, etc.
On the other hand, the current style widget is a fairly complex object that contains a lot of specialized logic, etc. It has to track state and drive a lot of behavior. However, it still can do it's job no matter where in the UI it is placed, as long as it is hooked to a specific desktop widget. It is a fairly good bit of architecture, and is very reusable and modular.
Also I think *how* the current style widget is updated has a good example. Instead of "peeking out" via SP_ACTIVE_DOCUMENT() or such, the widget has it's dependency "pushed in" via its setDesktop() call. In theory we could do a simple floating dialog that creates and holds a single current style widget. That floating dialog could listen for the "active" desktop to change and then when it hears an update that dialog can turn around and "push down" into the current style widget by calling setDesktop() on it. That's the kind of approach I'm trying to look at.
On Mar 2, 2010, at 7:49 AM, bulia byak wrote:
For user setting persistence we have preferences - it's better to add whatever functionality you need to them. The logic to determine where the toolbars should be by default based on screen resolution can be put somewhere in the toolbar creation code.
Definitely, all UI for this feature, whatever it is, should be in Preferences.
Oh, I forgot the mention the complications that popped up as I started to implement things:
Keys
Menus
Those are both configured by external XML files that are separate from the preferences. Ideally that would all be manageable in a single file/store but things might be a bit too complicated for that. I do know that switching to a "beginner mode" or an "illustrator mode" are top requested things in this area, so menus and keys should be configurable with all else together.
But, yes, for this iteration I was trying to keeps things to a minimum and just to what can fit easily in prefs.
On 3/3/10, Jon Cruz wrote:
things might be a bit too complicated for that. I do know that switching to a "beginner mode" or an "illustrator mode" are top requested things in this area, so menus and keys should be configurable with all else together.
While working on related stuff, could you please fix the bloody disgusting thing called "Why does Inkscape not tell me what shortcuts are used for every tool in their tooltips?"
Alexandre
participants (5)
-
Alexandre Prokoudine -
bulia byak -
Jon Cruz -
Joshua A. Andler -
Krzysztof Kosiński