View Issue Details

IDProjectCategoryView StatusLast Update
0000546FreeCADFeaturepublic2011-12-31 17:54
ReporterjmaustpcAssigned Towmayer 
PrioritynormalSeverityfeatureReproducibilityN/A
Status closedResolutionfixed 
Product Version 
Target VersionFixed in Version0.13 
Summary0000546: Ellipse added to create primitive dialogue
DescriptionI have added create Ellipse from centre and 2 point to the create primitive dialogue in Part WB.

I have tested it and it seems to work but I am going to check over this code again tomorrow, its nearly 3am here now. So I need a fresh mind to check it again for any obvious bugs.

TagsNo tags attached.

Activities

2011-12-18 14:51

 

svn_diff_FreeCAD_trunk_5314_add_ellipse_primitive_dlg_partwb.patch (Attachment missing)

jmaustpc

2011-12-19 01:05

manager   ~0001490

Last edited: 2011-12-19 01:07

seems to work as expected - would someone mind committing this?

wmayer

2011-12-19 11:50

administrator   ~0001492

Last edited: 2011-12-19 11:52

You must assign your report to someone (e.g. always to me) so that the developer gets an email notification, otherwise it can take some time before a developer goes through the bug/feature/patch list.

I had a quick look at the ellipse implementation and I consider this as problematic. Actually it is sufficient that the Ellipse feature only defines MinorRadius and MajorRadius as properties. All others are redundant and thus should be avoided. Especially the two axes are very problematic because they have a dependency to each other, namely they are perpendicular to each other.

So, the center and axes properties should be removed because this can also be expressed by the placement.

jmaustpc

2011-12-19 14:18

manager   ~0001495

About the assigning, no worries Werner, I didn't know I could assign, I thought you had to have to be given commit rights or some other rights/permissions or something.

But now I know I will do so next time. :-)


On the implementation, OK, I'll change it to work like the box/cylinder/etc., is that what you want? I thought that was dumbing it down to much.

I read a heap of the source code (OCE) there are about 6 or so different ways/sets of parameters, to implement this, I picked this, because it was slightly better for something I was wanting to draw, but mostly because it was slightly more difficult to code, hence was a challenge.

I will do as you asked and resubmit soon. But a question wouldn't it be best to give the user the option of doing it both/either/all ways?

I was thinking something along the lines of click here for ellipse, then select which creation method you want. Have default values for the parameters, but mostly leave them accessible for the odd time that it could be needed.


Jim

wmayer

2011-12-19 15:27

administrator   ~0001497

> About the assigning, no worries Werner, I didn't know I could assign, I thought you had to have to be given commit rights or some other rights/permissions or something.
NP. You don't need special permissions therefore. Just switch from simple to advanced view and you'll see the Assign box.

> On the implementation, OK, I'll change it to work like the box/cylinder/etc., is that what you want?
Yes, basically the same logic. So, for an ellipse you only have to have two controls for minor and major radius.

> I will do as you asked and resubmit soon.
For me it's sufficient that you only send a patch for the .ui file, the other files I can easily do it myself.

> But a question wouldn't it be best to give the user the option of doing it both/either/all ways?
Option for what? If you mean to define axes and center by the user then the answer is 'No'. This is almost because it may lead to some inconsistencies.

> I was thinking something along the lines of click here for ellipse, then select which creation method you want. Have default values for the parameters, but mostly leave them accessible for the odd time that it could be needed.
Yes/no/sort of :)
For the moment we should just do it the simple way. Of course we should offer alternative ways of creating a primitive. But therefore the primitive dialog must be re-organized first.

BTW, can you please try thr -b or -w option for the diff command? Currently your patch removes and adds a huge block of code due to some CR I guess.

jmaustpc

2011-12-19 16:03

manager   ~0001498

I will do the above,

But I can't assign anything, it won't let me either on creation or on later viewing.

my account says
Access Level reporter
Project Access Level reporter

I put this on the forum as I thought it would relevant info for others.

wmayer

2011-12-31 17:54

administrator   ~0001529

Added in rev 5371

Issue History

Date Modified Username Field Change
2011-12-18 14:51 jmaustpc New Issue
2011-12-18 14:51 jmaustpc File Added: svn_diff_FreeCAD_trunk_5314_add_ellipse_primitive_dlg_partwb.patch
2011-12-19 01:05 jmaustpc Note Added: 0001490
2011-12-19 01:07 jmaustpc Note Edited: 0001490
2011-12-19 11:50 wmayer Note Added: 0001492
2011-12-19 11:52 wmayer Note Edited: 0001492
2011-12-19 14:18 jmaustpc Note Added: 0001495
2011-12-19 15:27 wmayer Note Added: 0001497
2011-12-19 16:03 jmaustpc Note Added: 0001498
2011-12-20 09:58 wmayer Status new => assigned
2011-12-20 09:58 wmayer Assigned To => wmayer
2011-12-31 17:54 wmayer Note Added: 0001529
2011-12-31 17:54 wmayer Status assigned => closed
2011-12-31 17:54 wmayer Resolution open => fixed
2011-12-31 17:54 wmayer Fixed in Version => 0.13