Hi Martin!
* The parsing should also be perfomed by a non-UI layer and
not in
GimpUnitEntry so the parsing code can be used in e.g. a script
interpreter with no UI dependencies.
Parsing is done via GimpEevl which is called from the UnitEntry. I
think it makes sense because the entry is responsible for input and
output, the UnitAdjustment holds the value. GimpEevl does not
require any UI. Or did you mean our get_unit method? Maybe it would
make sense to modify GimpEevl to also determine the unit?
* GimpUnitEntryTable shouldn't derive from GtkTable, because
there is
no is-a relationship. To convince yourself of that: Can you
always
replace a GtkTable with a GimpUnitEntryTable? Nope, so you
should use
composition instead of inheritance.
Yeah maybe GimpUnitEntryTable could just be a set of (static)
functions that set up certain commonly used combinations of
UnitEntries, Labels etc. But i guess there is time to discuss that
in detail later, first I'd want to work on the UnitEntry itself ;)
* I've recomended a test-case based development approach
before, and
now that most of the code will have no UI-dependencies I
recomend it
even more, as non-UI classes generally are very easy to
unit-test. An
example of a commit that introduces a non-UI class along with
appropriate propotions of documentation and unit tests is this
commit:
http://git.gnome.org/browse/gimp/commit/?id=9fefa22efe70e484fc7c92708ed8efe023e4d219
By writing unit tests along with your code, productivity goes
up a lot
when you (or anyone in the lifetime of the project) refactor
code,
because verifying the class still works takes seconds.
Will look into that.
* Regarding the use case of entering "1 cm" and getting the
result in
px, we could use an 'in' keyword like Google. Analogous to
"100 SEK in
USD", we would have "1 cm in px". That is a future extension
though,
don't spend time on that yet.
Ok, should not be hard to implement this later.
* Have you thought about how the image resolution comes into
the picture?
Would it be ok to have our UnitAdjustment hold the resolution along
with the actual value? It does belong together. However, the problem
then would be if we want our entry to be used to input a resolution.
We could make that work by using just the resolution field of our
adjustment, but that would not be a very elegant solution. So
another class "GimpResolutionAdjustment"?
* Regarding your get_unit() method, an interesting question is
what
unit an _expression_ like "10px + 1cm" has.
I'd say use the first unit, px in that case. A user would enter an
_expression_ like that if
a) the 10 px are already in the entry and he just adds the "+1 cm",
so we can assume he wants the unit to stay px, or
b) he enters that term into an empty entry, in which case he could
have entered "1cm + 10px" if he wanted to have cm
* Have you thought about how to connect GimpUnitEntry to the
existing
GIMP code like the various props helper functions? (See how
GimpSizeEntry currently is used)
Have to look into that as well.
* I don't like how constraints currently are applied, in
particular
that you require two instance of a constraint when there is
only one
equation that shall hold. I don't think it is a good idea to
make
GimpUnitAdjustment (or GimpUnitEntry for that matter) be aware
of
constraints, because they can potentially be rather complex.
It is
probably a better idea to take care of constraints one one
architectural level above GimpUnitAdjustment (they don't know
about
constraints, constraints are just being applied to them).
* Regarding the contraint case, for a good design, consider
how you
would support constraints between three numbers (e.g. a = b +
c).
* Again, I would advice you to focus on the basics. Once the
basic
functionality is in place, meaning it is good enough for
inclusion in
a stable GIMP version, we can start working on constraints
support.
Yeah I guess I have to put a little bit more thought into that.
Something that comes to mind would be having a constraints class
using some kind of (regular) expressions. I'm thinking about
constraint.add(a, "a = b+c"), constraint.add(b, "b = c * 0.5a") and
so on. But that really would be something to do after Summer of
Code. I think the design would make it easy to add that later, so
i'll focus on the basics from now on ;)
* What program did you make those rather nice sequence
diagrams in?
OmniGraffle, you can make all sorts of diagrams with that, but as
far as I know it's mac-only.
Regards,
Enrico
Hi everybody,
I've come up with a updated (and more detailed) version of class
and sequence diagrams for the new widget.
http://enni.userpage.fu-berlin.de/GimpUnitEntry.pdf
I tried to incorporate some of the comments. Note that all names
are subject to change ;-)
Our now called GimpUnitEntry will be derived from GtkSpinScale
and use a subclass of GtkAdjustment to store its value including
the unit (GimpUnitAdjustment). All synchronisation and live
updating of associated UnitEntries will happen directly between
their GimpUnitAdjustments, thus completely separating everything
value-related from the gui and input. The GimpUnitEntry itself
will be just responsible for display and parsing of entered
terms. See use cases and class diagram for details.
Regards,
Enrico
2011/5/2 Enrico Schröder <enni.schroeder@xxxxxxxxx>:
Hi all,
i've come up with the first concept for the rewrite, including a class
diagram and sequence diagrams for a few use cases:
http://enni.userpage.fu-berlin.de/GimpSizeEntry.pdf
Note that it mainly shows how the different components work together,
not how each component does its work internally. If I forgot something
(and I probably have ;-) ) please tell me.
Martin: I planned on integration the keep-aspect-ratio functionality
right away, because I don't think it's to much additional work.
Hi Enrico
That's a good start! A couple of comments
* As this project is not about refactoring GimpSizeEntry but instead
write a new widget from scratch so we can get it right this time, we
need a new name. I could think of GimpDimensionEntry and
GimpUnitEntry, feel free to come up with something better.
* The sequence diagrams should be on the class interface i.e. method
level. For example, in the "simply entering two values" sequence
diagram, what classes and method calls will be involved in order to
let GimpSizeEntry b know about GimpSizeEntry a? (I don't understand
why in that use case they need to know about each others value at all
though, they are independent, aren't they?)
* You should put some thought into what exactly happens during
"enters value a", "enter value in a new unit" etc. You'll get a
GtkEditable::insert-text signal, but then what? Some kind of parsing
needs to take place. Take a look at GimpEevl which is a unit parser we
already have, resuing that would be ideal.
* In the "Changing aspect ratio" sequence diagram, a good design
would have an abstraction for the aspect ratio constraint, so that it
would be equally easy to have a constraint between two entries that
said "entry_a = entry_b + 100" as it is to have "entry_a = entry_b *
1.33". Think a base class GimpUnitConstraint with GimpOffsetContrainst
and GimpAspectRatioConstraint sub classes. In order to verify a design
for the aspect ratio preservation use case, what GimpUnitEntry classes
and method calls are involved in the following situation (which is the
main use case for aspect ratio preservation):
The current image has a pixel size of 200x100. The user does Image ->
Scale Image that brings up a dialog with two GimpUnitEntries, one for
width and one for height. The user can toggle between preserving and
not preserving aspect ratio. With aspect ratio preserved, the user
focuses Width (= 200) and erases one of the zeroes. At the same time,
the Height (= 100) entry changes to "10". What method calls were
involved to make that happen? When you have a sequence diagram that
answers that, you have a design.
But, don't put too much time into aspect ratio preservation. In fact,
I would prefer if you put as little effort as possible into this right
now (except making sure not to make it impossible to extend the design
with it later). Let me explain why: There are a lot of things that
could be done on GimpUnitEntry. Let's list a few things:
A The basic use case 'Enter a string in the form "<number> <unit>"
and have an interface that allows the pixel value with a given
resolution to be returned.
B The GimpSizeEntryTable you talked about
C Aspect ratio preservation between two GimpUnitEntries
At the end of the project, it is much better if you are 100% done with
A, and 0% on B and C, than if you hare 60% done with A and 20% done
with B and C. Code that is not delivered during the end of a GSoC
project has a tendency to either take a long time to hit upstream or
never does it at all. So we should work incrementally, first focus on
the basic use case A, then we can spend time on B and C.
To summarize, there are some things to sort out before we can say we
have a design. Once we have a design, we can start looking at writing
code. Now, of course, we probably won't get the design 100% right the
first time, it's an iterative process, but we should at least have an
initial design before we start coding.
Additionally I set up a task schedule on
http://tasktaste.com/projects/enni/gimpsizeentry and applied for a gnome
git account, but it probably takes some time for it to be activated.
Good, being able to track progress is essential if we want this to be
a successful GSoC project. I do think however that you should increase
the resolution of the tasks. It will be hard to follow up progress on
an 8 week big task. Let's settle on an initial design before creating
more detailed tasks though.
Also, since I'm using a Mac and tried to not having to use a virtual
machine, I built git-gimp natively on osx (without X11) and with a patch
that moves the menubar from the main window to the top of the screen
(like other mac apps). It really was a horrible experience (took me a
whole day), so I thought it would be nice to have a precompiled
app-bundle. As far as I know, there are no official mac binaries, right?
The only ones I found where using X11, which isn't very good. I could
try to provide osx binaries of the current 2.7.2 and then 2.8 including
patches for the menu bar and a nice theme.
Not quite sure I follow, how would precompiled binaries help? You'll
need to compile the code yourself anyway,won't you? Btw, as soon as we
have a feature branch, I am going to set up our continuous integration
server Jenkins (http://gimptest.flamingtext.com:8080/) to build
nightly tarballs of your work. That way it will be rather easy for
anyone to test your code.
Regards,
Martin
Hi all,
i've come up with the first concept for the rewrite, including a
class diagram and sequence diagrams for a few use cases:
http://enni.userpage.fu-berlin.de/GimpSizeEntry.pdf
Note that it mainly shows how the different components work
together, not how each component does its work internally. If I
forgot something (and I probably have ;-) ) please tell me.
Martin: I planned on integration the keep-aspect-ratio
functionality right away, because I don't think it's to much
additional work.
Additionally I set up a task schedule on
http://tasktaste.com/projects/enni/gimpsizeentry and applied for
a gnome git account, but it probably takes some time for it to
be activated.
Also, since I'm using a Mac and tried to not having to use a
virtual machine, I built git-gimp natively on osx (without X11)
and with a patch that moves the menubar from the main window to
the top of the screen (like other mac apps). It really was a
horrible experience (took me a whole day), so I thought it would
be nice to have a precompiled app-bundle. As far as I know,
there are no official mac binaries, right? The only ones I found
where using X11, which isn't very good. I could try to provide
osx binaries of the current 2.7.2 and then 2.8 including patches
for the menu bar and a nice theme.
Regards,
Enrico
|
