Shape tools create new shape layers: Bug 299844

[Sven Langkamp]

On Wed, Jul 4, 2012 at 6:52 PM, Michael O'Sullivan
<francium.duck at gmail.com>wrote:

> On 4 July 2012 11:00, Michael O'Sullivan <francium.duck at gmail.com> wrote:
> >
> > On Jul 3, 2012 9:07 PM, "Sven Langkamp" <sven.langkamp at gmail.com> wrote:
> >>
> >> On Tue, Jul 3, 2012 at 9:07 PM, Michael O'Sullivan
> >> <francium.duck at gmail.com> wrote:
> >>>
> >>> Hello!
> >>>
> >>> I'm very impressed with Krita, and would like to contribute. I've only
> >>> started looking at the source code today though, and I'm now feeling
> >>> rather stupid.
> >>>
> >>> The scratch I'd like to itch is the following bug:
> >>> Bug 299844 - using a shape tool creates new shape layer even if shape
> >>> layer selected
> >>> https://bugs.kde.org/show_bug.cgi?id=299844
> >>>
> >>> I've found that the "draw a path" and "draw straight line with current
> >>> brush" tools don't seemed to be influenced by this bug, so I was
> >>> hoping to compare and contrast the implementations of these tools with
> >>> those of the ellipse and rectangle shape tools. The only problem is,
> >>> I'm not sure which files the "draw a path" and "draw straight line
> >>> with current brush" tools are implemented in the krita/ui/tools
> >>> directory. Can anyone point me in the right direction?
> >>>
> >>
> >> Welcome!
> >>
> >> krita/ui/tools are just the base classes for tools. The specific tools
> are
> >> under krita/plugins/tools.
> >> The line and path tool are under krita/plugins/tools/defaulttools
> >> kis_tool_line and kis_tool_path
> >>
> >> You might also have a look at ui/kis_shape_controller.cpp there is a
> >> method addShape that adds the new shape layer.
> >>
> >> If you have questions you can also ask in the Krita IRC channel #krita
> on
> >> irc.freenode.net
> >>
> >> Good luck!
> >>
> >> _______________________________________________
> >> kimageshop mailing list
> >> kimageshop at kde.org
> >> https://mail.kde.org/mailman/listinfo/kimageshop
> >>
> >
> > Thank you so much! This may take me a little while; I'm still working out
> > how it all fits together. I'll lurk on the irc channel :)
> >
> > Michael
>
> Hi there,
>
> Following Sven's helpful pointers I've been playing around with the
> source. I'm afraid there's still lots I don't understand.
>
> In the addShape method in ui/flake/kis_shape_controller.cpp there is a bool
>
> static inline bool belongsToShapeSelection(KoShape* shape) {
>     return dynamic_cast<KisShapeSelectionMarker*>(shape->userData());
> }
>
> that the method uses as a condition for the first if statement.
>
> if (belongsToShapeSelection(shape))
> // etc
>
> I don't understand what the method does with the returned pointer.
>
>
The code simply checks if shape->userData() is a KisShapeSelectionMarker by
interpreting the result of the dynamic cast as bool. If that's the case it
knows that the shape belongs to a shape selection and not to a shape layer.


> Following the program logic, it seems that using the rectangle and
> ellipse tool return a false to this first if statement anyway and
> instead we go to the else condition:
>
> } else {
>         KisShapeLayer *shapeLayer =
> dynamic_cast<KisShapeLayer*>(shape->parent());
>
>         if (!shapeLayer) {
>             KoShapeLayer *rootLayer =
> shapeForNode(image()->rootLayer().data());
>             shapeLayer = new KisShapeLayer(rootLayer, this, image(),
>                                            i18n("Vector Layer %1",
> m_d->nameServer->number()),
>                                            OPACITY_OPAQUE_U8);
>
>             image()->undoAdapter()->addCommand(new
> KisImageLayerAddCommand(image(), shapeLayer, image()->rootLayer(),
> image()->rootLayer()->childCount()));
>         }
>
>         shapeLayer->addShape(shape);
>     }
>
> So at the moment I'm working on the basis that the rectangle and
> ellipse tools are not setting a parent for the shape, despite a vector
> laying being selected upon drawing. By changing the string parameter
> for the new KisShapeLayer, I've established the if (!shapeLayer)
> condition evaluates as true for this bug. I've been going round and
> round in circles in the source and I'm not quite sure how to fix that.
>

There are two possible cases. Either the parent is not set or it's not a
KisShapeLayer. In both cases the dynamic cast fails and shapeLayer is 0.


> I can see, for example, in the ellipse tool constructor there is a
> KoCanvasBase pointer parameter; and that the ellipse tool inherits
> from KisToolShape, which has an addShape method that uses the canvas
> parameter.
>
> I'm not sure how to set a shape parent from the canvas pointer (or
> even if this is the right approach). For instance, does the canvas
> pointer correlate to a layer anyway?
>

The parent isn't explicitly set in the tool, but if a shape get added it
will automatically get a parent.

You could check if the parent gets set, if it's the right parent or if
there is a difference in the parent between a working and a non-working
tool.
Do you know how to use a debugger? You could simply put a breakpoint on
line where the shape gets added in the tool and then step though the code
from there.

I'm probably trying to run before I can walk (or even crawl) at the
> moment, so apologies if this is all stupid! If anyone can give me an
> additional shove in the right direction that would be super!
>
> Many thanks,
>
> Michael
> _______________________________________________
> kimageshop mailing list
> kimageshop at kde.org
> https://mail.kde.org/mailman/listinfo/kimageshop
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kimageshop/attachments/20120704/970ea1d5/attachment.html>