2013/8/27 Arshdeep Singh <moduli16@...400...>:
I added: new (&(priv->nodes)) std::mapstd::string,RecolorWheelNode(); to recolor_wheel_init (RecolorWheel *wheel) .
Honestly I have never seen this style of code, so I really don't understand what it does. The good news is it does get to run the dialog box of 'Recolor Artwork' i.e. no error while the dialog is loaded, but the moment I draw a shape (active selection) the application crashes.
This 'style of code' is known as 'placement new' and it calls the constructor of an object with an explicit 'this' pointer. It needs to be used when memory for an object is allocated from a different source than operator new.
In normal C++, placement new is rarely used, because objects are typically allocated and constructed at the same time with operator new. However, if you allocate memory from a different source (in this case from GObject), you need to call the constructor yourself.
http://en.wikipedia.org/wiki/Placement_syntax
Also my init function looks like this (for a quick browse, if needed):
static void recolor_wheel_init (RecolorWheel *wheel) {
RecolorWheelPrivate *priv;
priv = G_TYPE_INSTANCE_GET_PRIVATE (wheel, RECOLOR_TYPE_COLOR_WHEEL, RecolorWheelPrivate);
wheel->priv = (RecolorWheelPrivate*)priv; priv = (RecolorWheelPrivate*)wheel->priv ;
What is the point of the last line? Why the casts? They are unnecessary.
What I don't undersytnd is how to initilaize a static object. RecolorWheelPrivate has other static members like :
What exactly do you mean by "static members"? The structure you provided does not define any static members.
gdouble h,s,v don't need explicit initializers. Why so ? Is it that the only map object requires an explicit initilization. If so, why ?
'gdouble' is a primitive type (a typedef of 'double') which doesn't have any constructors. GObject zeroes memory by default, so if you don't assign to h, s, v they are left as all zeroes, which corresponds to the double value of 0.0.
Regards, Krzysztof
On 08/27/2013 10:56 PM, Krzysztof Kosiński wrote:
2013/8/27 Arshdeep Singh <moduli16@...400...>:
I added: new (&(priv->nodes)) std::mapstd::string,RecolorWheelNode(); to recolor_wheel_init (RecolorWheel *wheel) .
Honestly I have never seen this style of code, so I really don't understand what it does. The good news is it does get to run the dialog box of 'Recolor Artwork' i.e. no error while the dialog is loaded, but the moment I draw a shape (active selection) the application crashes.
This 'style of code' is known as 'placement new' and it calls the constructor of an object with an explicit 'this' pointer. It needs to be used when memory for an object is allocated from a different source than operator new.
In normal C++, placement new is rarely used, because objects are typically allocated and constructed at the same time with operator new. However, if you allocate memory from a different source (in this case from GObject), you need to call the constructor yourself.
Be careful with placement new onto GObjects allocated by g_object_new. g_object_new puts some GObject type info into the first few bytes of the struct which is trashed when placement new invokes the whole constructor hierarchy beginning at GObject (the C++ class) since this class' constructor can not preserve the type info field's memory content. You will not notice that in normal operation except when using dynamic GObject type casts (those CLASS_NAME(object) macros) which will throw warnings on the console, though not crash, or when using g_object_unref, since the reference counter is initialized with 1 but overwritten to 0. Also, at many points in Inkscape's code base there are asserts that will crash Inkscape when they encounter a GObject lacking type information.
Best, Sebastian
2013/8/28 Sebastian Götte <jaseg@...2974...>:
Be careful with placement new onto GObjects allocated by g_object_new. g_object_new puts some GObject type info into the first few bytes of the struct which is trashed when placement new invokes the whole constructor hierarchy beginning at GObject (the C++ class) since this class' constructor can not preserve the type info field's memory content. You will not notice that in normal operation except when using dynamic GObject type casts (those CLASS_NAME(object) macros) which will throw warnings on the console, though not crash, or when using g_object_unref, since the reference counter is initialized with 1 but overwritten to 0. Also, at many points in Inkscape's code base there are asserts that will crash Inkscape when they encounter a GObject lacking type information.
You should never call placement new on the GObject itself. But you have to call it on its members.
Hi, I think the problem lies in the fact that performUpdate() is called many times when we are dragging the nodes on the wheel. That is why, the computationally intensive snippet that I posted a few mails ago in this thread is causing a crash ? Can that be a reason ? If you take a look at the snippet, you'll understand what I mean.
And I really going by Krzysztof's reply, the placement new should not cause a crash the way I have used it in my code. Correct ?
On Wed, Aug 28, 2013 at 8:01 PM, Krzysztof Kosiński <tweenk.pl@...400...>wrote:
2013/8/28 Sebastian Götte <jaseg@...2974...>:
Be careful with placement new onto GObjects allocated by g_object_new.
g_object_new puts some GObject type info into the first few bytes of the struct which is trashed when placement new invokes the whole constructor hierarchy beginning at GObject (the C++ class) since this class' constructor can not preserve the type info field's memory content. You will not notice that in normal operation except when using dynamic GObject type casts (those CLASS_NAME(object) macros) which will throw warnings on the console, though not crash, or when using g_object_unref, since the reference counter is initialized with 1 but overwritten to 0. Also, at many points in Inkscape's code base there are asserts that will crash Inkscape when they encounter a GObject lacking type information.
You should never call placement new on the GObject itself. But you have to call it on its members.
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more! Discover the easy way to master current and previous Microsoft technologies and advance your career. Get an incredible 1,500+ hours of step-by-step tutorial videos with LearnDevNow. Subscribe today and save! http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clk... _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
2013/8/28 Arshdeep Singh <moduli16@...400...>:
Hi, I think the problem lies in the fact that performUpdate() is called many times when we are dragging the nodes on the wheel. That is why, the computationally intensive snippet that I posted a few mails ago in this thread is causing a crash ? Can that be a reason ? If you take a look at the snippet, you'll understand what I mean.
No, doing a lot of computations never leads to a crash.
And I really going by Krzysztof's reply, the placement new should not cause a crash the way I have used it in my code. Correct ?
Yes, the placement new you added is correct and your crash is caused by something else. The "use after free" messages you are getting indicate that your code is doing something wrong with pointers and accessing unallocated regions of memory.
Regards, Krzysztof
Inkscape::Selection *selection = sp_desktop_selection(desktop); GSList const *items = NULL; RecolorWheel* wheel = (RecolorWheel*) (((RecolorWheelSelector*)(rsel))->getWheel()) ; RecolorWheelNode temp;
g_printf("\nWe are here: RecolorArtworkWidget::selectionModifiedCB() ! ");
if ( selection ) { items = selection->itemList();
for (GSList const *i = items; i != NULL; i = i->next) { SPObject *obj=reinterpret_cast<SPObject *>(i->data); Inkscape::XML::Node* obj_repr = obj->getRepr(); SPCSSAttr* obj_css = sp_repr_css_attr( obj_repr , "style" );
guint32 rgb32 = sp_svg_read_color( sp_repr_css_property( obj_css, "fill", "#ababab") , 0xF0F8FF ); SPColor color = SPColor (rgb32);
float rgb[3] , hsv[3]; sp_color_get_rgb_floatv (&color, rgb); sp_color_rgb_to_hsv_floatv (hsv , temp._color[0] , temp._color[1] , temp._color[2] );
add_node_to_recolor_wheel (wheel, obj->getId() , temp );
}
}
Do you anything done in this snippet above is wrong ? Because commenting this out leads to no crashes at all. The moment this is included, everything goes to hell.
On Wed, Aug 28, 2013 at 9:25 PM, Krzysztof Kosiński <tweenk.pl@...400...>wrote:
2013/8/28 Arshdeep Singh <moduli16@...400...>:
Hi, I think the problem lies in the fact that performUpdate() is called many times when we are dragging the nodes on the wheel. That is why, the computationally intensive snippet that I posted a few mails ago in this thread is causing a crash ? Can that be a reason ? If you take a look at
the
snippet, you'll understand what I mean.
No, doing a lot of computations never leads to a crash.
And I really going by Krzysztof's reply, the placement new should not
cause
a crash the way I have used it in my code. Correct ?
Yes, the placement new you added is correct and your crash is caused by something else. The "use after free" messages you are getting indicate that your code is doing something wrong with pointers and accessing unallocated regions of memory.
Regards, Krzysztof
2013/8/28 Arshdeep Singh <moduli16@...400...>:
RecolorWheel* wheel = (RecolorWheel*) (((RecolorWheelSelector*)(rsel))->getWheel()) ;
This is a forced cast. Change this to GObject casts, e.g.
RecolorWheel *wheel = RECOLOR_WHEEL(RECOLOR_WHEEL_SELECTOR(rsel)->getWheel());
This way if you made a mistake here, you'll get a runtime warning.
add_node_to_recolor_wheel (wheel, obj->getId() , temp );
The problem might also be inside this function.
Regards, Krzysztof
These are the two functions:
void add_node_to_recolor_wheel (RecolorWheel *wheel, std::string name, RecolorWheelNode node) { RecolorWheelPrivate *priv = (RecolorWheelPrivate*) (wheel->priv); (priv->nodes).insert( std::pairstd::string,RecolorWheelNode(name,node) ); }
void remove_node_to_recolor_wheel (RecolorWheel *wheel, std::string name) { RecolorWheelPrivate *priv = (RecolorWheelPrivate*) wheel->priv;
std::mapstd::string,RecolorWheelNode::iterator iter; iter = priv->nodes.find(name); priv->nodes.erase( iter ); }
On Wed, Aug 28, 2013 at 9:38 PM, Krzysztof Kosiński <tweenk.pl@...972.....>wrote:
2013/8/28 Arshdeep Singh <moduli16@...400...>:
RecolorWheel* wheel = (RecolorWheel*) (((RecolorWheelSelector*)(rsel))->getWheel()) ;
This is a forced cast. Change this to GObject casts, e.g.
RecolorWheel *wheel = RECOLOR_WHEEL(RECOLOR_WHEEL_SELECTOR(rsel)->getWheel());
This way if you made a mistake here, you'll get a runtime warning.
add_node_to_recolor_wheel (wheel, obj->getId() , temp );The problem might also be inside this function.
Regards, Krzysztof
I guess you are correct Krzysztof. Making the 'add_node_to_recolor_wheel' function empty stops Inkscape from crashing. So the problem has to lie in the casting. I have also tried :
RecolorWheelPrivate *priv; priv = G_TYPE_INSTANCE_GET_PRIVATE (wheel, RECOLOR_TYPE_COLOR_WHEEL, RecolorWheelPrivate); // followed by the code (priv->nodes).insert( std::pairstd::string,RecolorWheelNode(name,node) );
but this again leads to a crash.
Finally, atleast the brickwall is clear to us. Now all we need is a road roller :) so the function crashing Inkscape is:
void add_node_to_recolor_wheel (RecolorWheel *wheel, std::string name, RecolorWheelNode node) { RecolorWheelPrivate *priv = (RecolorWheelPrivate*) (wheel->priv); (priv->nodes).insert( std::pairstd::string,RecolorWheelNode(name,node) ); }
On Wed, Aug 28, 2013 at 10:33 PM, Arshdeep Singh <moduli16@...400...> wrote:
These are the two functions:
void add_node_to_recolor_wheel (RecolorWheel *wheel, std::string name, RecolorWheelNode node) { RecolorWheelPrivate *priv = (RecolorWheelPrivate*) (wheel->priv); (priv->nodes).insert( std::pairstd::string,RecolorWheelNode(name,node) ); }
void remove_node_to_recolor_wheel (RecolorWheel *wheel, std::string name) { RecolorWheelPrivate *priv = (RecolorWheelPrivate*) wheel->priv;
std::map<std::string,RecolorWheelNode>::iterator iter; iter = priv->nodes.find(name); priv->nodes.erase( iter );}
On Wed, Aug 28, 2013 at 9:38 PM, Krzysztof Kosiński <tweenk.pl@...847...0...>wrote:
2013/8/28 Arshdeep Singh <moduli16@...400...>:
RecolorWheel* wheel = (RecolorWheel*) (((RecolorWheelSelector*)(rsel))->getWheel()) ;
This is a forced cast. Change this to GObject casts, e.g.
RecolorWheel *wheel = RECOLOR_WHEEL(RECOLOR_WHEEL_SELECTOR(rsel)->getWheel());
This way if you made a mistake here, you'll get a runtime warning.
add_node_to_recolor_wheel (wheel, obj->getId() , temp );The problem might also be inside this function.
Regards, Krzysztof
-- Arshdeep Singh Third Year, Computer Engineering Delhi Technological University Ph: +91-9654115614 https://sites.google.com/site/adsingh1729/home
participants (4)
-
Arshdeep Singh -
Kris De Gussem -
Krzysztof Kosiński -
Sebastian Götte