How to properly fix bug and commit to Inkscape trunk
Dear Inkscape-devel,
I would like to fix a simple bug I found in src/helper/geom.cpp::pathv_to_cubicbezier(). The control points are not set correctly:
Geom::CubicBezier b(cit->initialPoint(), cit->pointAt(0.3334) + Geom::Point(cubicGap,cubicGap), cit->finalPoint(), cit->finalPoint());
where it should have been:
Geom::CubicBezier b(cit->initialPoint(), cit->pointAt(0.3333) + Geom::Point(cubicGap,cubicGap), cit->pointAt(0.6667) + Geom::Point(cubicGap,cubicGap), cit->finalPoint());
or more precisely:
Geom::CubicBezier b(cit->initialPoint(), cit->pointAt(1./3.) + Geom::Point(cubicGap,cubicGap), cit->pointAt(2./3.) + Geom::Point(cubicGap,cubicGap), cit->finalPoint());
(The Geom::Point(cubicGap,cubicGap) shouldn’t really be there, but that’s a separate issue I described in another email.)
I am not sure how to properly submit this. Do I follow the practice for bzr here http://wiki.inkscape.org/wiki/index.php/Working_with_Bazaar#Test_branches_before_committing_merges and simply merge my fix (without a need for someone to review)?
Regards, _______________________ Papoj "Hua" Thamjaroenporn papojt@...3117...
Hi Papoj. I do the original function :( and your fix is good,. Anyway when you create a new branch in launchpad web you can mark the branch for review.
All the best, Jabier.
El mié, 16-12-2015 a las 21:28 -0500, Papoj Thamjaroenporn escribió:
Dear Inkscape-devel,
I would like to fix a simple bug I found in src/helper/geom.cpp::pathv_to_cubicbezier(). The control points are not set correctly:
Geom::CubicBezier b(cit->initialPoint(), cit-
pointAt(0.3334) + Geom::Point(cubicGap,cubicGap), cit->finalPoint(),
cit->finalPoint());
where it should have been:
Geom::CubicBezier b(cit->initialPoint(), cit-
pointAt(0.3333) + Geom::Point(cubicGap,cubicGap), cit- pointAt(0.6667) + Geom::Point(cubicGap,cubicGap), cit- finalPoint());
or more precisely:
Geom::CubicBezier b(cit->initialPoint(), cit-
pointAt(1./3.) + Geom::Point(cubicGap,cubicGap), cit->pointAt(2./3.)
- Geom::Point(cubicGap,cubicGap), cit->finalPoint());
(The Geom::Point(cubicGap,cubicGap) shouldn’t really be there, but that’s a separate issue I described in another email.)
I am not sure how to properly submit this. Do I follow the practice for bzr here http://wiki.inkscape.org/wiki/index.php/Working_with_Ba zaar#Test_branches_before_committing_merges and simply merge my fix (without a need for someone to review)?
Regards, _______________________ Papoj "Hua" Thamjaroenporn papojt@...3117...
Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Thank you Jabier. No need for the frown, because I am very happy that I can finally contribute something!
Regards, _______________________ Papoj "Hua" Thamjaroenporn papojt@...3117...
On Dec 17, 2015, at 4:11 AM, Jabier Arraiza <jabier.arraiza@...2893...> wrote:
Hi Papoj. I do the original function :( and your fix is good,. Anyway when you create a new branch in launchpad web you can mark the branch for review.
All the best, Jabier.
El mié, 16-12-2015 a las 21:28 -0500, Papoj Thamjaroenporn escribió:
Dear Inkscape-devel,
I would like to fix a simple bug I found in src/helper/geom.cpp::pathv_to_cubicbezier(). The control points are not set correctly:
Geom::CubicBezier b(cit->initialPoint(), cit-pointAt(0.3334) + Geom::Point(cubicGap,cubicGap), cit->finalPoint(),
cit->finalPoint());
where it should have been:
Geom::CubicBezier b(cit->initialPoint(), cit-pointAt(0.3333) + Geom::Point(cubicGap,cubicGap), cit- pointAt(0.6667) + Geom::Point(cubicGap,cubicGap), cit- finalPoint());
or more precisely:
Geom::CubicBezier b(cit->initialPoint(), cit-pointAt(1./3.) + Geom::Point(cubicGap,cubicGap), cit->pointAt(2./3.)
- Geom::Point(cubicGap,cubicGap), cit->finalPoint());
(The Geom::Point(cubicGap,cubicGap) shouldn’t really be there, but that’s a separate issue I described in another email.)
I am not sure how to properly submit this. Do I follow the practice for bzr here http://wiki.inkscape.org/wiki/index.php/Working_with_Ba zaar#Test_branches_before_committing_merges and simply merge my fix (without a need for someone to review)?
Regards, _______________________ Papoj "Hua" Thamjaroenporn papojt@...3117...
Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Hi Papoj. offtopic. When reply to all I get this mail error mesage:
<pt2277@...3110...>: host mail-in.cc.columbia.edu[128.59.105.70] said: 550 5.7.1 Access denied (in reply to MAIL FROM command)
Reporting-MTA: dns; mx.marker.es X-mx-marker-es-Queue-ID: 1AD05A1238 X-mx-marker-es-Sender: rfc822; javier.arraiza@...2893... Arrival-Date: Thu, 17 Dec 2015 09:27:14 +0000 (UTC)
Final-Recipient: rfc822; pt2277@...3110... Original-Recipient: rfc822;pt2277@...3110... Action: failed Status: 5.7.1 Remote-MTA: dns; mail-in.cc.columbia.edu Diagnostic-Code: smtp; 550 5.7.1 Access denied
El jue, 17-12-2015 a las 04:14 -0500, Papoj Thamjaroenporn escribió:
Thank you Jabier. No need for the frown, because I am very happy that I can finally contribute something!
Regards, _______________________ Papoj "Hua" Thamjaroenporn papojt@...3117...
On Dec 17, 2015, at 4:11 AM, Jabier Arraiza <jabier.arraiza@...3311... es> wrote:
Hi Papoj. I do the original function :( and your fix is good,. Anyway when you create a new branch in launchpad web you can mark the branch for review.
All the best, Jabier.
El mié, 16-12-2015 a las 21:28 -0500, Papoj Thamjaroenporn escribió:
Dear Inkscape-devel,
I would like to fix a simple bug I found in src/helper/geom.cpp::pathv_to_cubicbezier(). The control points are not set correctly:
Geom::CubicBezier b(cit->initialPoint(), cit-
pointAt(0.3334) + Geom::Point(cubicGap,cubicGap), cit-
finalPoint(),
cit->finalPoint());
where it should have been:
Geom::CubicBezier b(cit->initialPoint(), cit-
pointAt(0.3333) + Geom::Point(cubicGap,cubicGap), cit- pointAt(0.6667) + Geom::Point(cubicGap,cubicGap), cit- finalPoint());
or more precisely:
Geom::CubicBezier b(cit->initialPoint(), cit-
pointAt(1./3.) + Geom::Point(cubicGap,cubicGap), cit-
pointAt(2./3.)
- Geom::Point(cubicGap,cubicGap), cit->finalPoint());
(The Geom::Point(cubicGap,cubicGap) shouldn’t really be there, but that’s a separate issue I described in another email.)
I am not sure how to properly submit this. Do I follow the practice for bzr here http://wiki.inkscape.org/wiki/index.php/Working_wit h_Ba zaar#Test_branches_before_committing_merges and simply merge my fix (without a need for someone to review)?
Regards, _______________________ Papoj "Hua" Thamjaroenporn papojt@...3117...
Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
[ resending to the list ]
On 2015-12-17 03:28 (+0100), Papoj Thamjaroenporn wrote:
I am not sure how to properly submit this. Do I follow the practice for bzr here http://wiki.inkscape.org/wiki/index.php/Working_with_Bazaar#Test_branches_before_committing_merges and simply merge my fix (without a need for someone to review)?
For small changes/fixes, you can also file a bug report (please include info about OS/platform and Inkscape version/revision in the description), with a brief summary of the encountered issue (maybe a test case) and the proposed solution, and attach a diff against latest trunk (generated with bzr) to the bug report for review/testing:
https://bugs.launchpad.net/inkscape/+filebug
To create a diff of changes to your local branch (not committed), you can use e.g. this bazaar command:
$ bzr diff path/to/modified/file_or_directory > fix_this_issue.diff
Working with branches and merge proposals might often be preferred or more useful for larger changes (e.g. affecting many files), or for fixes which themselves have important commit history (merging bazaar branches will keep that history intact).
Regards, V
participants (3)
-
Jabier Arraiza -
Papoj Thamjaroenporn -
su_v