RSS/Atom feed Twitter
Site is read-only, email is disabled

A plan for GimpRectangleTool

This discussion is connected to the gimp-developer-list.gnome.org mailing list which is provided by the GIMP developers and not related to gimpusers.com.

This is a read-only list on gimpusers.com so this discussion thread is read-only, too.

3 of 3 messages available
Toggle history

Please log in to manage your subscriptions.

A plan for GimpRectangleTool Martin Nordholts 25 Jun 08:46
  A plan for GimpRectangleTool Simon Budig 25 Jun 10:44
   A plan for GimpRectangleTool Martin Nordholts 25 Jun 21:27
Martin Nordholts
2007-06-25 08:46:56 UTC (over 17 years ago)

A plan for GimpRectangleTool

Hello

There are now 24 bugs that have a 2.4 milestone and 6 of these are related or directly uses the code in app/tools/gimprectangletool.c.

As most of you know the code in there is a mess. In the last few days I've been thinking about a way to clean up this mess, and I have a plan I'd like to throw out there in an attempt to synchronize the direction we are taking the code in.

With the introduction of helper functions like

void gimp_rectangle_tool_constrain_aspect (GimpRectangleTool *rectangle_tool, gdouble aspect, const gchar *apply_on_x, const gchar *apply_on_y)
{
/* apply_on_x may be "x1", "x2" or "center_x", same for on_y */ }

and verbose comments and member/variables names, it would be possible to apply these modifications on the rectangle in different order depending on the state of GimpRectangleOptions

We would also avoid weaving in different logic on the same line of code, which currently is what the code in gimp_rectangle_tool_motion is horribly close to doing.

For a taste of what I mean, checkout this patch: http://bugzilla.gnome.org/attachment.cgi?id=90541&action=view

attached to this bug: http://bugzilla.gnome.org/show_bug.cgi?id=398183 (CTRL modifier on a selection does not snap the starting corner back...)

It introduces e.g.

void gimp_rectangle_tool_set_other_side_coord (GimpRectangleTool *rectangle_tool, gint other_side_x, gint other_side_y)

and verbose commenting.

Personally I intend to realize this plan in the following weeks, I just wanted to throw it out to the mailing list. Partly because, as I said, to synchronize the direction the code is taking, but also to get feedback and to avoid clashes with other Big Plans.

- Martin Nordholts

Simon Budig
2007-06-25 10:44:40 UTC (over 17 years ago)

A plan for GimpRectangleTool

Martin Nordholts (enselic@gmail.com) wrote:

With the introduction of helper functions like

void gimp_rectangle_tool_constrain_aspect (GimpRectangleTool *rectangle_tool, gdouble aspect, const gchar *apply_on_x, const gchar *apply_on_y)
{
/* apply_on_x may be "x1", "x2" or "center_x", same for on_y */ }

Hmm, wouldn't an enum for apply_on_* make more sense?

Bye, Simon

Martin Nordholts
2007-06-25 21:27:54 UTC (over 17 years ago)

A plan for GimpRectangleTool

Simon Budig skrev:

Martin Nordholts (enselic@gmail.com) wrote:

With the introduction of helper functions like

void gimp_rectangle_tool_constrain_aspect (GimpRectangleTool *rectangle_tool, gdouble aspect, const gchar *apply_on_x, const gchar *apply_on_y)
{
/* apply_on_x may be "x1", "x2" or "center_x", same for on_y */ }

Hmm, wouldn't an enum for apply_on_* make more sense?

Bye, Simon

Well, that will depend on if it will be used similarly to the gimp_rectangle_tool_set_other_side-family of functions that were in the patch I linked to, or, perhaps more likely, if it will be used on the local x1, x2, etc gint:s in gimp_rectangle_tool_motion. I'd prefer the former but we'll see how the logic in _motion folds out.

- Martin