Bug 161327 - LibreOffice does not handle dr3d:end-angle values correctly
Summary: LibreOffice does not handle dr3d:end-angle values correctly
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Not Assigned
URL: https://docs.oasis-open.org/office/Op...
Whiteboard: odf target:24.8.0
Keywords:
Depends on:
Blocks: ODF-import 3D-Model ODF-export-invalid
  Show dependency treegraph
 
Reported: 2024-05-29 18:55 UTC by Regina Henschel
Modified: 2024-06-11 09:47 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Henschel 2024-05-29 18:55:59 UTC
Start a new Draw document.
Draw a diagonal line.
Convert the line to 3D Rotation Object (from context menu).
Select scene if not already selected and open the "3D Effects" dialog (from context menu).
Enter the scene. The rotation object should be selected, otherwise select it.
Set line style to "solid".
In "3D Effects" dialog set field "Rotation Angle" to 180°.
Exit scene.
Save document, e.g. to .fodg.

Open the saved file in an editor.
Search for the attribute "dr3d:end-angle".
It has the value 1800. That is wrong.

ODF specifies, that the angle has datatype angle (18.3.1).
Open specification there. https://docs.oasis-open.org/office/OpenDocument/v1.3/os/part3-schema/OpenDocument-v1.3-os-part3-schema.html#__RefHeading__1416540_253892949

You read there:
<quote>
An angle, as defined in §4.1 of [SVG], is a double value that may be followed immediately by one of the following angle unit identifiers: deg (degrees), grad (gradiants) or rad (radians). If no unit identifier is specified, the value is assumed to be in degrees.
</quote>

As LibreOffice does not write a unit, the value should be 180, not 1800.

Now we try reading that value. In editor set instead of the "1800" any of "180deg", "200grad" or "3.14159265359rad".
Open the so changed document. In any case you get a rotation object with 360°. That is, LibreOffice cannot interpret a unit identifier and uses the default 360° instead.

I think, the solution can be done in two steps.
1. Make LibreOffice able to interpret an angle unit identifier for the dr3d:end-angle.
2. When writing to ODF 1.4, write "180deg" instead of "1800".

I think, we should keep interpreting a unit-less value in documents of ODF 1.3 and earlier as being 1/10 degree for to be backward-compatible with older documents.

Michael, what do you think about the way to fix the problem?
Comment 1 Regina Henschel 2024-05-29 19:01:36 UTC
In "3D Effects" dialog set field "Rotation Angle" to 180°. _Apply_
Comment 2 m_a_riosv 2024-05-30 10:58:00 UTC
I can see the value '1800' on the raw file (fodg), but reopening the file and calling 3D effects, shows the value '180º'

Version: 24.8.0.0.alpha1+ (X86_64) / LibreOffice Community
Build ID: 6084962f93efc005b6827edceae12d3170f17ccd
CPU threads: 16; OS: Windows 11 X86_64 (10.0 build 22631); UI render: Skia/Raster; VCL: win
Locale: es-ES (es_ES); UI: en-US
Calc: CL threaded
Comment 3 Regina Henschel 2024-05-30 12:55:10 UTC
(In reply to m_a_riosv from comment #2)
> I can see the value '1800' on the raw file (fodg), but reopening the file
> and calling 3D effects, shows the value '180º'

That is why we need a way to stay backward compatible. LibreOffice interprets the value as being 1/10 degree, but ODF clearly specifies it to be in degree.

I cannot estimate whether the technical committee would agree to change the specification so that values without a unit are interpreted as 1/10 degree. I don't know if there are any applications other than LibreOffice and OpenOffice that can display 3D objects.
Comment 4 m_a_riosv 2024-05-30 13:25:08 UTC
I don't have the technical acknowledge, to look in the code, but perhaps the last '0' is interpreted as a default value -> degrees, not really a 1/10 value is done.
Comment 5 Michael Stahl (allotropia) 2024-06-04 10:40:26 UTC
the problem sounds familiar...
Comment 6 Michael Stahl (allotropia) 2024-06-04 11:03:01 UTC
i think bug 89475 was only about gradients.

there are also these "angle" in the schema:
* draw:start-angle/draw:end-angle attribute on "common-draw-circle-ellipse-attlist"
* style:rotation-angle on "common-rotation-angle-attlist"
* dr3d:shadow-slant on "dr3d-scene-attlist"
* draw:text-rotate-angle, draw:extrusion-rotation-angle, draw:extrusion-skew on "draw-enhanced-geometry-attlist"
* draw:rotation on "draw-hatch-attlist"
* chart:angle-offset on "style-chart-properties-attlist"
* draw:caption-angle on "style-graphic-properties-attlist"
* style:text-rotation-angle on "style-text-properties-attlist"

i hope these aren't all broken?
Comment 7 Regina Henschel 2024-06-04 11:54:13 UTC
(In reply to Michael Stahl (allotropia) from comment #6)
 
> i hope these aren't all broken?

I will investigate it.
Comment 8 Regina Henschel 2024-06-05 20:20:53 UTC
My investigation (see below) shows, that we need a more general approach as currently in https://gerrit.libreoffice.org/c/core/+/168336

Perhaps the following?
Add
bool Converter::convertAngleWithUnit(double& rAngle, std::u16string_view rString, bool& rHasUnit)
bool Converter::convertAngleWithUnit(double& rAngle, std::string_view rString, bool& rHasUnit)
and use it where currently convertNumber or convertDouble is used for angles.

Not only converting the angle with unit to degree, but inform whether there was a unit, allows to use it in Converter::convertAngle and for the cases (dr3d:end-angle and draw:rotation in hatch) where the wrongly written 1/10th of a degree has to be considered.

So make all cases able to read angles with unit. 

In cases were currently unit less degree is written, we need not change the export but only the import.

In cases were currently unit less 1/10th of a degree is written, we will need to export unit 'deg' at some time, when all active versions are able to read units. ODF 1.4 is likely too early, but ODF 1.5 will work.


And these are the results of my investigation (hopefully correct):

None of the cases is able to import angle with units.
None of the cases exports angle units.

(A) writes unit-less 1/10th of a degree
    -----------------------------------
    draw:rotation => UNO FillHatch.Angle
    dr3d:end-angle => UNO D3DEndAngle (this bugreport)

(B) export: unit-less degree
    import: ignores units, takes number part as degree
    --------------------------------------------------
    common-draw-circle-ellipse-attlist
        draw:start-angle => UNO CircleStartAngle
        draw:end-angle => UNO CircleEndAngle

    common-rotation-angle-attlist
        chart:angle-offset => UNO StartingAngle

    in charts, e.g. in title and axis label
        style:rotation-angle => UNO TextRotation

    dr3d-scene-attlist
        dr3d:shadow-slant => UNO D3DSceneShadowSlant

    draw-enhanced-geometry-attlist
        draw:text-rotate-angle => UNO TextRotateAngle (in CustomShapeGeometry)
        draw:extrusion-rotation-angle => UNO RotateAngle (CustomShapeGeometry.Extrusion)
        draw:extrusion-skew => UNO Skew (in CustomShapeGeometry.Extrusion)

    draw:caption-angle on "style-graphic-properties-attlist"
        draw:caption-angle => UNO CaptionAngle (can be set/get in macros, but has no effect in rendering)

(C) export: unit-less degree
    import: number with unit is not imported at all, results in default 0°
    ----------------------------------------------------------------------
    table-cell-properties-attlist
        style:rotation-angle => UNO RotateAngle

(D) export: unit-less degree
    import: unit-less degree values are mapped to 0, 90 and 270; others not supported
            number with unit is not imported at all, results in default value 0°
    -------------------------------------------------------------------------------------
    style-text-properties-attlist in character styles
        style:text-rotation-angle => UNO CharRotation

(E) style:text-rotation-angle in number format, list or outline is not implemented
Comment 9 Michael Stahl (allotropia) 2024-06-07 09:53:07 UTC
(In reply to Regina Henschel from comment #8)
> My investigation (see below) shows, that we need a more general approach as
> currently in https://gerrit.libreoffice.org/c/core/+/168336
> 
> Perhaps the following?
> Add
> bool Converter::convertAngleWithUnit(double& rAngle, std::u16string_view
> rString, bool& rHasUnit)
> bool Converter::convertAngleWithUnit(double& rAngle, std::string_view
> rString, bool& rHasUnit)
> and use it where currently convertNumber or convertDouble is used for angles.
> 
> Not only converting the angle with unit to degree, but inform whether there
> was a unit, allows to use it in Converter::convertAngle and for the cases
> (dr3d:end-angle and draw:rotation in hatch) where the wrongly written 1/10th
> of a degree has to be considered.
> 
> So make all cases able to read angles with unit. 

definitely angles with unit should be read everywhere.

one issue with these functions is that the SvXMLImport can check the version numbers, but it can't be used from "sax" module, so xmloff has to do it.

i think it would work equally well to have rHasUnit output parameter or isWrongOOo10thDegAngle and let the function convert it.

i'm wondering, is a new function for import required at all?
for the serious cases, until the export is fixed, you could just pass "true" as the isWrongOOo10thDegAngle for convertAngle()?

except if there are cases where more precision than 10th-of-degrees is supported, a "double" implementation would make sense for that.

> In cases were currently unit less degree is written, we need not change the
> export but only the import.
> 
> In cases were currently unit less 1/10th of a degree is written, we will
> need to export unit 'deg' at some time, when all active versions are able to
> read units. ODF 1.4 is likely too early, but ODF 1.5 will work.

 yes
 
> And these are the results of my investigation (hopefully correct):
> 
> None of the cases is able to import angle with units.
> None of the cases exports angle units.
> 
> (A) writes unit-less 1/10th of a degree
>     -----------------------------------
>     draw:rotation => UNO FillHatch.Angle
>     dr3d:end-angle => UNO D3DEndAngle (this bugreport)

okay good, so only one additional serious problem...
Comment 10 Regina Henschel 2024-06-09 16:01:53 UTC
I have opened bug 161483 for the cases where LibreOffice is not able to read angle units, but correctly writes unit-less angles in degree.

So this bug report is for the two cases in (A), where LibreOffice incorrectly writes angle values in 1/10 of a degree.

(In reply to Michael Stahl (allotropia) from comment #9)
> except if there are cases where more precision than 10th-of-degrees is
> supported, a "double" implementation would make sense for that.

The datatype 'angle' in ODF is a double. But discussion it easier, after I have uploaded the next version of my patch.
Comment 11 Commit Notification 2024-06-11 08:30:22 UTC
Regina Henschel committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/567dbcf68711402736f65f4a0a47ef57549fe50f

tdf#161327 Interpret angle units in dr3d:end-angle ..

It will be available in 24.8.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 12 Regina Henschel 2024-06-11 09:47:50 UTC
The part to write unit "deg" can only be done when all active versions are able to read "deg" and it is likely that users have switched to them. The code has some remarks on that. It might be useful to keep this bug report open to get an automatic remainder in two years in addition.

Thus I do not set it to "resolved fixed" but put it back to "new".