Shape tools create new shape layers: Bug 299844

Sven Langkamp sven.langkamp at gmail.com
Thu Jul 5 22:07:40 UTC 2012


On Thu, Jul 5, 2012 at 7:22 PM, Michael O'Sullivan
<francium.duck at gmail.com>wrote:

> On 4 July 2012 19:25, Sven Langkamp <sven.langkamp at gmail.com> wrote:
> > 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
> >
> >
> >
> > _______________________________________________
> > kimageshop mailing list
> > kimageshop at kde.org
> > https://mail.kde.org/mailman/listinfo/kimageshop
> >
>
> Hi there!
>
> Thanks very much for the reply Sven, it was very helpful. I followed
> the progress of the KisShapeController::addShape method with gdb using
> the Krita line tool (which works as expected) and the rectangle and
> ellipse tools (which create new vector layers). You are correct that
> the issue is with the dynamic cast: it is successful in the case of
> the line and path tool; in the case of the rectangle, etc, while there
> is a value for the parent, the dynamic cast fails and it is set to
> 0x0.
>
> At the moment I don't know why there is a difference between the two
> sets of tools. I haven't worked out what classes the respective
> parents belong to before the dynamic cast.
>
> Ultimately (I think) both sets of tools use the same addShape method
> from KoShapeController. The line and path tools apply that directly,
> whereas the rectangle and ellipse tools inherit an addShape method
> from KisToolShape, which itself applies the KoShapeController addShape
> method.
>
> // ...
> KUndo2Command * cmd = canvas()->shapeController()->addShape(shape);
>     canvas()->addCommand(cmd);
>

What I meant is that you start the the addShape call above and step into it
and see which path it takes through the KoShapeController. At some point it
will have to set the parent and there you can check what happens. Both
tools should do exactly the same so you can check step by step if there is
a different behavior. If it's not visible immediately step into each
function and check if there is a difference.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kimageshop/attachments/20120706/ac8bf50f/attachment-0001.html>


More information about the kimageshop mailing list