After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 728197 - Roundtrip for graphs
Roundtrip for graphs
Status: RESOLVED OBSOLETE
Product: Gnumeric
Classification: Applications
Component: Charting
git master
Other All
: Normal normal
: ---
Assigned To: Jean Bréfort
Jody Goldberg
Depends on:
Blocks:
 
 
Reported: 2014-04-14 17:09 UTC by Morten Welinder
Modified: 2018-05-22 14:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Sample patch (2.20 KB, patch)
2014-05-08 07:12 UTC, Jean Bréfort
none Details | Review
Updated patch (2.22 KB, patch)
2014-05-08 07:18 UTC, Jean Bréfort
none Details | Review
Updated graph-tests.gnumeric with marker tests (4.31 KB, application/x-gnumeric)
2014-05-10 20:12 UTC, Morten Welinder
  Details

Description Morten Welinder 2014-04-14 17:09:32 UTC
New test t6516 fails for all formats.
Comment 1 Jean Bréfort 2014-04-14 17:29:11 UTC
This one will be quite difficult to fix, if just possible.
Comment 2 Morten Welinder 2014-04-14 17:51:45 UTC
That's certainly possible.

However, note that like for some of the other tests we have the option
of telling the tests not to look at specific things.  Greek text in biff7,
for example.

The hope is to identify specific things such as things one for ods:

-                  <text_layout/>
+                  <text_layout angle="0"/>
Comment 3 Jean Bréfort 2014-04-14 18:19:28 UTC
We don't even roundtrip through our own format, apparently. Weird.
Comment 4 Morten Welinder 2014-04-14 21:00:10 UTC
I fixed the angle issue.

Yes, the lack of roundtrip for gnumeric is strange.
Comment 5 Morten Welinder 2014-04-15 01:16:25 UTC
The reason for the .gnumeric mismatches:

-                    <line dash="solid" auto-dash="1" width="0" color="0:0:80:FF" auto-color="1"/>
-                    <marker auto-shape="1" shape="diamond" auto-outline="1" outline-color="0:0:80:FF" auto-fill="1" fill-color="0:0:80:FF" size="5"/>
+                    <line dash="solid" auto-dash="1" width="0" color="0:0:0:FF" auto-color="1"/>
+                    <marker auto-shape="1" shape="none" auto-outline="1" outline-color="0:0:0:FF" auto-fill="1" fill-color="FF:FF:FF:FF" size="5"/>

is that we apply a theme on load to those items that are "auto".

This begs the question: why we are saving anything for those?
Comment 6 Jean Bréfort 2014-04-15 11:19:51 UTC
I don't know why we save auto fields. We always did as far as I know.

change of auto fields when going through xls or odt can't be avoided because the semantics of auto values is somewhat different in other spreadsheets.

Ths xlsx is not surprising, our chart export is very incomplete (I could never correctly export a radial axis style to xlsx and could not find why).
Comment 7 Morten Welinder 2014-04-15 15:32:27 UTC
We not longer save values for things that are "auto".  Gnumeric roundtrips.
Comment 8 Jean Bréfort 2014-04-16 06:23:39 UTC
We don't really need to save the "is-auto" properties, I suppose, although this would need some changes in the loading code to stay compatible.
Comment 9 Jean Bréfort 2014-04-16 13:38:38 UTC
Fixed the xls axis issue (I'm pretty sure there are others around, especially in the catserrange field).

The remaining issues in xls biff8 can't be fixed, or we might have the same file, but not the same appearance than in other spreadsheets (not 100% sure, but I fixed several related bugs years ago).
Comment 10 Andreas J. Guelzow 2014-04-17 02:58:37 UTC
<property name="manual-size">size</property>

Wouldn't it make more sense to have a boolean?
Comment 11 Andreas J. Guelzow 2014-04-17 03:16:33 UTC
My last comment was intended for a different bug...
Comment 12 Andreas J. Guelzow 2014-04-17 03:25:56 UTC
Re comment #7: I do not see that Gnumeric round trips. I see:

t6516-graph.pl: Check graph gnumeric roundtrip.
--- graph-tests.xml	2014-04-16 21:22:22.500258423 -0600
+++ graph-tests-new.xml	2014-04-16 21:22:22.508258423 -0600
@@ -99,7 +99,7 @@
             <property name="theme-name">Default</property>
             <property name="padding-pts">7.086614</property>
             <property name="style" type="GogStyle">
-              <outline auto-dash="1" width="0" auto-color="1"/>
+              <outline dash="none" auto-dash="1" width="0" color="0:0:0:FF" auto-color="1"/>
               <fill type="none" auto-type="1" is-auto="1" auto-fore="1"/>
             </property>
             <property name="anchor">top-left</property>

and many more.
Comment 13 Andreas J. Guelzow 2014-04-17 05:16:28 UTC
For ODF roundtrip we have:
         <gnm:SheetObjectGraph ObjectBound="B1:F9" ObjectOffset="0.516 0.412 0.656 0.0588" Direction="17" Print="1">
           <GogObject type="GogGraph">
-            <property name="height-pts">97.496700</property>
-            <property name="width-pts">198.720000</property>
             <property name="theme-name">Default</property>

Since the height-pts and width-pts can be calculated from 
ObjectBound="B1:F9" ObjectOffset="0.516 0.412 0.656 0.0588"
and the known column widths and row heights, this seems to be a redundant piece of information.

What do we gain by calculating this?
Comment 14 Morten Welinder 2014-04-17 12:08:01 UTC
Andreas: don't worry about height-pts etc.  We don't even get those from
xls so they can't be that important.

Strange observation: graph-tests.gnumeric and the xls file we create from
that both load with a graph that has a frame around it.  When we load the
ods file that either we or localc generates, we get no frame.  And yet
it's not showing the the roundtrip diffs in any way I can tell.
Comment 15 Morten Welinder 2014-04-18 00:35:07 UTC
I have masked the auto-dash issue for ods.
There still are some width issues as well as an auto-type issue.
Comment 16 Andreas J. Guelzow 2014-04-18 18:55:59 UTC
ODF roundtrips now.

Note that issue of a missing frame around the graph for ODF did show in the diff as the 'width' difference for the 'outline'.
Comment 17 Morten Welinder 2014-04-18 22:41:42 UTC
Great!  I'm likely to break it again when adding more stuff, but I want to
get the Excel situation under control first.
Comment 18 Andreas J. Guelzow 2014-04-18 23:56:54 UTC
We cannot handle the auto_* stuff round tripping through LO, but we can do it for round trips through ODF+extension. Since that is what we are really testing here, I have disabled that filter part and fixed the auto_dash issue.
Comment 19 Morten Welinder 2014-04-19 00:33:10 UTC
Works for me.

I have strategic goal of one day being able to supply

    --primary-format=ods

to configure -- not because I think ods is a particular good format, but
I can see an organisation wanting to standardize on it.

Having to use extensions is an unfortunate thing, but if nothing else it
can serve as a reminder about what parts of the ods standard we would like
to see updated one day.  Anyway, I'm just thinking ahead here.
Comment 20 Morten Welinder 2014-04-19 15:25:31 UTC
Something went wrong with the Y coordinates for xlsx:

-        <gnm:SheetObjectGraph ObjectBound="B1:F9" ObjectOffset="0.516 0.412 0.656 0.0588" Direction="17" Print="1">
+        <gnm:SheetObjectGraph ObjectBound="B1:F9" ObjectOffset="0.516 0.109 0.656 0.0134" Direction="17" Print="1">
Comment 21 Andreas J. Guelzow 2014-04-19 15:37:44 UTC
There is also some strange scaling happening in xlsx import for the horizontal direction. I added the corresponding scaling in export, but that means that the numbers may be wrong inside the xlsx file.
Comment 22 Morten Welinder 2014-04-19 23:44:41 UTC
The read side of xlsx object positions was badly broken in that it was
using columns for both columns and rows.
Comment 23 Morten Welinder 2014-04-22 18:14:01 UTC
I have added a few more things to t6516.

ods loses the colour of titles:

-                  <font color="0:0:FF:FF" font="Sans Bold 12" auto-scale="1"/>
+                  <font color="0:0:0:FF" font="Sans Bold 12" auto-scale="1"/>
Comment 24 Morten Welinder 2014-05-07 18:22:20 UTC
Several issues here:

1. go_style_font_sax_save doesn't save font.auto_color.

2. go_style_sax_load_font leaves font.auto_color unchanged.  We will need some
   kind of hack when reading old files that do not have the "auto-color"
   attribute.  Maybe "black is auto, others not."

With 1+2 fixed we can resave graph-tests and have the auto-flag reasonably
set (in memory).  Then...

3. odf_write_gog_style_graphic knows nothing about fonts.

Comments, anyone?
Comment 25 Jean Bréfort 2014-05-07 19:29:41 UTC
May be we should not save auto fields at all.
Comment 26 Morten Welinder 2014-05-07 20:00:16 UTC
As long as we keep theming support we need the auto fields.  I don't think
we want to get rid of it, even if we aren't using the theming code a lot
right now.
Comment 27 Jean Bréfort 2014-05-07 20:08:08 UTC
The idea was not exactly that: if a color is auto, we might not save the color. When reading, if a color is found then it is not auto (unless if the auto field is there, of course, for compatibility). Samthing for line width, dashes and other fields that might be auto.
Comment 28 Morten Welinder 2014-05-07 20:32:12 UTC
Ah, I see.

We can do that if we can figure out a way to do it while still being able
to read existing files.  For now I am going to move towards saving the
"auto" flag and only saving data for non-auto fields.  If we can get that
to work, then we can make the "auto" fields optional later.
Comment 29 Jean Bréfort 2014-05-08 07:12:46 UTC
Created attachment 276132 [details] [review]
Sample patch

Just a sample of my idea concerning lines only.
Comment 30 Jean Bréfort 2014-05-08 07:18:27 UTC
Created attachment 276134 [details] [review]
Updated patch
Comment 31 Jean Bréfort 2014-05-08 14:27:50 UTC
Actually, if we want the new files to be loaded correctly by old gnumeric instances, we need to save some auto flags, but only when they are FALSE since the default is to have all set to TRUE.
Comment 32 Morten Welinder 2014-05-09 01:04:58 UTC
We do want old gnumerics to read the new files.

And we worry somewhat over how new gnumerics read old files that lack
certain auto flags.
Comment 33 Morten Welinder 2014-05-09 01:39:46 UTC
I added saving of auto-width with a heuristic for reading old files.
Comment 34 Morten Welinder 2014-05-09 01:54:35 UTC
I added saving of font.auto-color with a heuristic for reading old files.
Comment 35 Morten Welinder 2014-05-09 15:43:11 UTC
Chart text colour is now exported and LO can load it.  We cannot.
Comment 36 Morten Welinder 2014-05-09 19:09:33 UTC
We roundtrip chart titles' colours now.
Comment 37 Morten Welinder 2014-05-10 20:12:23 UTC
Created attachment 276298 [details]
Updated graph-tests.gnumeric with marker tests

This updated file (which will be committed when I can) shows various problems:


1. We fail to export the size for the diamond-shaped marker.  All the others
   do get their sizes exported.

2. We fail to export marker colour.

I believe (2) is not fully solvable.  Or, if it is, it involves a little
svg file as the marker.  (svg to make it scalable.)

Other than as an image, I don't think that ods allows different colours for
the line and the markers.  Andreas, can you confirm that?

We have three different colours in play: line, marker, and marker's
outline.  Something will have to give.  In this case -- which has no
line at all -- I suspect that we should use the marker's fill colour
for the series.  Maybe, if the fill colours is fully transparent, we should
even go on to using the outline colour.
Comment 38 Morten Welinder 2014-05-10 21:00:02 UTC
Fix in hand for export-size problem.  The shape isn't "diamond", but "auto"
in this case.
Comment 39 Morten Welinder 2014-05-11 15:19:25 UTC
For ods we have a number of these:

-            <property name="padding-pts">7.086614</property>
+            <property name="padding-pts">7.086614173228346</property>

(The reason that we have a few non-default values, i.e., 7.086614 is that
we used to save doubles with too little precision; we now roundtrip this
truncated value.)

We get the difference above because we don't save this to ods at all.
Andreas: do we have a place to save this in ods?
Comment 40 Morten Welinder 2014-05-11 15:31:07 UTC
this change is peculiar:

-            <property name="height-pts">92.90700000000004</property>
+            <property name="height-pts">87.92175000000003</property>

It looks like we're adjusting for some kind of padding on write, but not
on read.
Comment 41 Morten Welinder 2014-05-14 18:05:03 UTC
I made the padding-pts part of the file use the default value, so we don't
see any differences until we start setting it on purpose.

I have hidden differences related to outline colour.

We are down to this:

+                  <property name="interpolation-skip-invalid">TRUE</property>


-                    <marker auto-shape="1" auto-outline="1" auto-fill="1" size="5"/>
+                    <marker auto-shape="0" shape="none" auto-outline="1" auto-fill="1" size="5"/>


-            <property name="height-pts">92.90700000000004</property>
+            <property name="height-pts">87.92175000000003</property>

-            <property name="height-pts">85.49924999999999</property>
+            <property name="height-pts">89.7345</property>


(And that's really not bad!)
Comment 42 Andreas J. Guelzow 2014-05-16 02:01:35 UTC
+                  <property name="interpolation-skip-invalid">TRUE</property>
 is now fixed
Comment 43 Andreas J. Guelzow 2014-05-16 02:40:27 UTC
It is strange that there are only 2 occurrences of height-pts differences, since there are many more charts in the file.

for the last one we have:

        <gnm:SheetObjectGraph ObjectBound="C11:F17" ObjectOffset="0.0625 0.235 0.641 0.882" Direction="17" Print="1">
          <GogObject type="GogGraph">
            <property name="height-pts">85.49924999999999</property>
            <property name="width-pts">171.76800000000003</property>
            <property name="theme-name">Default</property>
            <property name="padding-pts">7.086614173228346</property>


and saved in ODF:

            <draw:frame svg:x="3pt" svg:y="3.1725pt" table:end-x="30.768pt" table:end-y="11.907pt" svg:width="171.76800000000003pt" svg:height="89.7345pt" table:end-cell-address="$'Markers'.F17" draw:z-index="16">
Comment 44 Morten Welinder 2014-05-16 23:00:00 UTC
+                  <property name="interpolation-skip-invalid">TRUE</property>

I still see that.  Missing "push"?


> It is strange that there are only 2 occurrences of height-pts differences,
> since there are many more charts in the file.

I agree.  And changing something -- anything -- for the graph in question
tends to make the difference go away.


This is new and comes from new tests I added:

-                    <marker auto-shape="0" shape="circle" auto-outline="0" outline-color="0:FF:0:FF" auto-fill="0" fill-color="0:FF:0:FF" size="5"/>
+                    <marker auto-shape="1" auto-outline="0" outline-color="0:0:0:FF" auto-fill="0" fill-color="FF:FF:FF:FF" size="5"/>
Comment 45 Andreas J. Guelzow 2014-05-17 00:09:24 UTC
Yes, there was a push missing. It should be gone now.
Comment 46 Andreas J. Guelzow 2014-05-17 02:46:33 UTC
This seems to be a problem with reading the ODF file, since we have there:

<style:style style:name="GOG-0x18df120" style:family="chart">
      <style:chart-properties chart:interpolation="gnm:cspline" gnm:interpolation-skip-invalid="false" chart:lines="true" chart:symbol-name="circle" chart:symbol-width="5pt" chart:symbol-height="5pt" chart:symbol-type="named-symbol"/>
      <style:graphic-properties gnm:auto-type="true" draw:stroke="dash" draw:stroke-dash="s-dot" gnm:auto-width="true" svg:stroke-color="#00ff00"/>
      <style:paragraph-properties/>
      <style:text-properties fo:font-size="8pt" fo:font-variant="normal" fo:font-family="Sans" fo:font-style="normal" fo:font-weight="normal" gnm:font-stretch-pango="4"/>
    </style:style>

So the shape of the marker is given (correctly). We apparently are not reading it correctly.
Comment 47 Andreas J. Guelzow 2014-05-17 03:34:07 UTC
The reason while we don't get the correct marker is the code

 		if (gnm_object_has_readable_prop (state->chart.plot,
						  "default-style-has-markers",
						  G_TYPE_BOOLEAN,
						  &dshm) &&
		    !dshm) {
			style->marker.auto_shape = TRUE;
			go_marker_set_shape (m, GO_MARKER_NONE);
		}

in odf_apply_style_props.
Comment 48 Morten Welinder 2014-05-18 14:54:12 UTC
Marker issue fixed.

The basic problem is that we cannot export the marker as "automatic"
when that means "none" as it does in a plot that doesn't have markers by
default.  We have to export as "none" and deduce the automatic.
Comment 49 Morten Welinder 2014-05-19 14:31:24 UTC
Interestingly we roundtrip the trend line I added -- module some naming -- but
localc doesn't see the trend line at all.
Comment 50 Andreas J. Guelzow 2014-05-19 18:19:12 UTC
The file we are creating is not valid ODF 1.2 (extended):

graph-tests.ods/Graph30/content.xml[106,235]: Error: attribute "chart:interpolation" has a bad value. Possible values are: b-spline,cubic-spline,none
Comment 51 Andreas J. Guelzow 2014-05-19 18:30:23 UTC
The issue of comment #50 is a gnumeric specific attribute value. According to 3.17 in ODF 1.2 part 1:

A conforming consumer that encounters an OpenDocument defined attribute that has a value that is not defined by OpenDocument, then it should:
1) If the attribute has a specified default value, use its default value, or
2) If the attribute does not have a specified default value, ignore the attribute.

Moreover we have there:
OpenDocument extended documents may contain elements and attributes not defined by the OpenDocument schema. Elements and attributes not defined by the OpenDocument schema are called foreign elements and attributes.

Note that this does not allow for foreign attribute values. We should use a foreign attribute.
Comment 52 Andreas J. Guelzow 2014-05-19 19:02:57 UTC
The issue of comment #50 is fixed.
Comment 53 Andreas J. Guelzow 2014-05-19 19:07:01 UTC
When LO saves the trendline, it includes the specs to the style attached to the series. We are attaching that info to the style of the chart:regression-curve element, which I believe is more correct.

LO apparently ignores the specification if attached to the style of the chart:regression-curve element. I believe that is an LO bug.
Comment 54 Andreas J. Guelzow 2014-05-21 06:47:05 UTC
I have filed that LO bug as https://bugs.freedesktop.org/show_bug.cgi?id=78999
Comment 55 Andreas J. Guelzow 2014-05-21 07:54:25 UTC
Supposedly LO 4.2 and later is showing the trendline.
Comment 56 Morten Welinder 2014-05-23 22:59:19 UTC
I can confirm that my 4.2 does show the trendline.
Comment 57 GNOME Infrastructure Team 2018-05-22 14:10:27 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnumeric/issues/253.