Bug 131044 - 3D Effects dialog does not allow to set 1 vertical segment
Summary: 3D Effects dialog does not allow to set 1 vertical segment
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Draw (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium minor
Assignee: Not Assigned
URL:
Whiteboard: target:7.0.0
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-01 00:17 UTC by Regina Henschel
Modified: 2020-03-04 13:21 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Example to examine the number of vertical segments (47.15 KB, application/vnd.oasis.opendocument.graphics)
2020-03-01 00:17 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Henschel 2020-03-01 00:17:36 UTC
Created attachment 158282 [details]
Example to examine the number of vertical segments

Look into attached file. It has for the rotation object the attribute dr3d:vertical-segments="1" in style "gr2". That is correct, because the path of the object is a single line.

Now open the file in Draw and open the "3D Effects" dialog. It has a cone. The line style is set to "solid" so that the segments are visible.
Click on the object.
Change something in the dialog, e.g. set angle to 270°. Assign it.
Now the cone has no longer 1 vertical segment but 2.
Go back to the dialog and try to change the number of vertical segments back to 1. It is not possible.

It is valid for a rotation object to have 1 vertical segments. If the underlying path is a polyline, then the vertical segments correspond to the number of line segments in the path. And in case the path has only a single line segment the number in 'vertical segments' should be 1 too.
Comment 1 Julien Nabet 2020-03-01 20:41:42 UTC
Just for information, with this patch:
diff --git a/svx/uiconfig/ui/docking3deffects.ui b/svx/uiconfig/ui/docking3deffects.ui
index d08c981ac984..bfa45fe5befa 100644
--- a/svx/uiconfig/ui/docking3deffects.ui
+++ b/svx/uiconfig/ui/docking3deffects.ui
@@ -50,7 +50,7 @@
     <property name="page_increment">10</property>
   </object>
   <object class="GtkAdjustment" id="adjustment9">
-    <property name="lower">2</property>
+    <property name="lower">1</property>
     <property name="upper">256</property>
     <property name="step_increment">1</property>
     <property name="page_increment">10</property>

vertical displays "1".
(adjustement9 is related to vertical segments with:
    494                       <object class="GtkSpinButton" id="veri">
    495                         <property name="visible">True</property>
    496                         <property name="can_focus">True</property>
    497                         <property name="text">0</property>
    498                         <property name="adjustment">adjustment9</property>
    499                       </object>
)
Comment 2 Xisco Faulí 2020-03-02 11:33:39 UTC
Moving to NEW

@Julien, Do you plan to submit the patch to gerrit ?
Comment 3 Julien Nabet 2020-03-02 12:50:41 UTC
(In reply to Xisco Faulí from comment #2)
> Moving to NEW
> 
> @Julien, Do you plan to submit the patch to gerrit ?

It was more a code pointer because I don't know why it's been put at 2 for min.
So don't hesitate to submit a patch on gerrit if you want and know the impact.
Comment 4 Julien Nabet 2020-03-02 12:51:33 UTC
(In reply to Julien Nabet from comment #3)
> (In reply to Xisco Faulí from comment #2)
> > Moving to NEW
> > 
> > @Julien, Do you plan to submit the patch to gerrit ?
> 
> It was more a code pointer because I don't know why it's been put at 2 for
> min.
> So don't hesitate to submit a patch on gerrit if you want and know the
> impact.

Also, what about horizontal part? There's also "2" for min, should it be changed to "1" ?
Comment 5 Regina Henschel 2020-03-02 13:25:46 UTC
(In reply to Julien Nabet from comment #4)
> Also, what about horizontal part? There's also "2" for min, should it be
> changed to "1" ?

I don't know, whether horizontal "1" has any impacts. I know no scenario, which generates such object. And I see no use case for it. In contrast to vertical, where the user can generate such object.
Comment 6 Julien Nabet 2020-03-03 21:26:25 UTC
Patch waiting for review here:
https://gerrit.libreoffice.org/c/core/+/89933
Comment 7 Commit Notification 2020-03-04 08:01:40 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/3d38514166b2e3090e03d16df11e83e184eee433

tdf#131044: 3D Effects dialog should allow to set 1 vertical segment

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Xisco Faulí 2020-03-04 09:29:27 UTC
Just for the record, the dialog was welded by https://cgit.freedesktop.org/libreoffice/core/commit/?id=278f01eadd513608e306ea6b85d52fb115a6dedf
Comment 9 Caolán McNamara 2020-03-04 10:41:09 UTC
wrt comment comment #8, my understanding is that the min of 2 precedes my involvement, and checking openoffice.org that does seem to be the case
Comment 10 Xisco Faulí 2020-03-04 10:44:12 UTC
(In reply to Caolán McNamara from comment #9)
> wrt comment comment #8, my understanding is that the min of 2 precedes my
> involvement, and checking openoffice.org that does seem to be the case

Yep, not saying it was introduced by the welding process.
@Julien, would it be possible to backport it to 6-4 branch ?
Comment 11 Julien Nabet 2020-03-04 10:46:50 UTC
(In reply to Xisco Faulí from comment #10)
> (In reply to Caolán McNamara from comment #9)
> > wrt comment comment #8, my understanding is that the min of 2 precedes my
> > involvement, and checking openoffice.org that does seem to be the case
> 
> Yep, not saying it was introduced by the welding process.
> @Julien, would it be possible to backport it to 6-4 branch ?

There's a conflict and I don't know why.
=> unassign myself.
Comment 12 Xisco Faulí 2020-03-04 10:49:25 UTC
Because of the welding process. anyway, the issue has been around since forever, no real need for the backport. Should we close it ?
Comment 13 Julien Nabet 2020-03-04 10:55:32 UTC
(In reply to Xisco Faulí from comment #12)
> Because of the welding process. anyway, the issue has been around since
> forever, no real need for the backport. Should we close it ?

I let Regina decide, I've got no opinion here.
Comment 14 Regina Henschel 2020-03-04 13:21:40 UTC
No backport.