Re: [Inkscape-devel] Modifications to RecolorWheelPrivate
Hi,
Recap: I made a separate container for RecolorWheel.
Initially there was an initialization error that crashed the application. As pointed out by Krzysztof, the std::map<std::string, RecolorWheelNode> had to be explicitly initialized which made the crash go away. The Recolor Dialog box renders perfectly. Invoking the DialogBox in Inkscape was causing a crash (before the explicit initialization of map) . Unfortunately, the application still crashes when I draw a new object on the canvas. Basically the crash happens when I select an object on the canvas.
I was debugging the errors in gdb using g_message but here is the tricky part: When I run the inkscape.exe without gdb the application crashes only when the RecolorArtwork Dialog is active and an object on the canvas is selected. But, in gdb the application starts constantly giving the warning: " HEAP: Free heap block a90b8f6 modified at a90b8f6 after it was freed. "
As a result the application doesn't get to the point where I can invoke RecolorArtwork and debug it.
My latest branch can be found at: https://code.launchpad.net/~moduli16/inkscape/recolor .
Regards, Arshdeep
On Wed, Aug 28, 2013 at 2:38 AM, Arshdeep Singh <moduli16@...400...> wrote:
By static I meant 'non pointer' variables. Okay so this doesn't seem to be the problem causing the crash. Gdb ends up showing the same heap error I started this thread with. Can you spot an error as to why that keeps happening ? On Aug 28, 2013 2:25 AM, "Krzysztof Kosiński" <tweenk.pl@...400...> 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.
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
There is good news. I found out the area of code causing the crash (though not the reason). It was the snippet inside RecolorArtworkWidget::performUpdate() :
Inkscape::Selection *selection = NULL; selection = sp_desktop_selection(desktop); GSList const *items = NULL; RecolorWheel* wheel = (RecolorWheel*) (((RecolorWheelSelector*)(rsel))->getWheel()) ; RecolorWheelNode temp;
g_message("\nWe are here: updateFromPaint() ! ");
if ( selection!=NULL ) { items = selection->itemList(); g_message("\nWe are here: if(selection) ->performUpdate() ! ");
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", "") , 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 );
}
}
Can you think of a reason why ? I commented this out and everything seems to workout just fine, no crashes and all.
On Wed, Aug 28, 2013 at 1:12 PM, Arshdeep Singh <moduli16@...400...> wrote:
Hi,
Recap: I made a separate container for RecolorWheel.
Initially there was an initialization error that crashed the application. As pointed out by Krzysztof, the std::map<std::string, RecolorWheelNode> had to be explicitly initialized which made the crash go away. The Recolor Dialog box renders perfectly. Invoking the DialogBox in Inkscape was causing a crash (before the explicit initialization of map) . Unfortunately, the application still crashes when I draw a new object on the canvas. Basically the crash happens when I select an object on the canvas.
I was debugging the errors in gdb using g_message but here is the tricky part: When I run the inkscape.exe without gdb the application crashes only when the RecolorArtwork Dialog is active and an object on the canvas is selected. But, in gdb the application starts constantly giving the warning: " HEAP: Free heap block a90b8f6 modified at a90b8f6 after it was freed. "
As a result the application doesn't get to the point where I can invoke RecolorArtwork and debug it.
My latest branch can be found at: https://code.launchpad.net/~moduli16/inkscape/recolor .
Regards, Arshdeep
On Wed, Aug 28, 2013 at 2:38 AM, Arshdeep Singh <moduli16@...400...>wrote:
By static I meant 'non pointer' variables. Okay so this doesn't seem to be the problem causing the crash. Gdb ends up showing the same heap error I started this thread with. Can you spot an error as to why that keeps happening ? On Aug 28, 2013 2:25 AM, "Krzysztof Kosiński" <tweenk.pl@...400...> 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.
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
-- Arshdeep Singh Third Year, Computer Engineering Delhi Technological University Ph: +91-9654115614 https://sites.google.com/site/adsingh1729/home
The gdb still crashes constantly giving that HEAP warning though. Im debugging using g_printf( ) to a text file. :( Regards, Arshdeep
On Wed, Aug 28, 2013 at 5:04 PM, Arshdeep Singh <moduli16@...400...> wrote:
There is good news. I found out the area of code causing the crash (though not the reason). It was the snippet inside RecolorArtworkWidget::performUpdate() :
Inkscape::Selection *selection = NULL; selection = sp_desktop_selection(desktop); GSList const *items = NULL; RecolorWheel* wheel = (RecolorWheel*) (((RecolorWheelSelector*)(rsel))->getWheel()) ; RecolorWheelNode temp;
g_message("\nWe are here: updateFromPaint() ! "); if ( selection!=NULL ) { items = selection->itemList(); g_message("\nWe are here: if(selection) ->performUpdate() ! "); 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", "") , 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 ); } }Can you think of a reason why ? I commented this out and everything seems to workout just fine, no crashes and all.
On Wed, Aug 28, 2013 at 1:12 PM, Arshdeep Singh <moduli16@...400...>wrote:
Hi,
Recap: I made a separate container for RecolorWheel.
Initially there was an initialization error that crashed the application. As pointed out by Krzysztof, the std::map<std::string, RecolorWheelNode> had to be explicitly initialized which made the crash go away. The Recolor Dialog box renders perfectly. Invoking the DialogBox in Inkscape was causing a crash (before the explicit initialization of map) . Unfortunately, the application still crashes when I draw a new object on the canvas. Basically the crash happens when I select an object on the canvas.
I was debugging the errors in gdb using g_message but here is the tricky part: When I run the inkscape.exe without gdb the application crashes only when the RecolorArtwork Dialog is active and an object on the canvas is selected. But, in gdb the application starts constantly giving the warning: " HEAP: Free heap block a90b8f6 modified at a90b8f6 after it was freed. "
As a result the application doesn't get to the point where I can invoke RecolorArtwork and debug it.
My latest branch can be found at: https://code.launchpad.net/~moduli16/inkscape/recolor .
Regards, Arshdeep
On Wed, Aug 28, 2013 at 2:38 AM, Arshdeep Singh <moduli16@...400...>wrote:
By static I meant 'non pointer' variables. Okay so this doesn't seem to be the problem causing the crash. Gdb ends up showing the same heap error I started this thread with. Can you spot an error as to why that keeps happening ? On Aug 28, 2013 2:25 AM, "Krzysztof Kosiński" <tweenk.pl@...1063....> 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.
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
-- Arshdeep Singh Third Year, Computer Engineering Delhi Technological University Ph: +91-9654115614 https://sites.google.com/site/adsingh1729/home
-- Arshdeep Singh Third Year, Computer Engineering Delhi Technological University Ph: +91-9654115614 https://sites.google.com/site/adsingh1729/home
participants (1)
-
Arshdeep Singh