[SRU] scaling is wrong in SVG output

Bug #2015017 reported by David Huggins-Daines
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
graphviz (Debian)
New
Unknown
graphviz (Ubuntu)
Fix Released
Undecided
Unassigned
Jammy
Fix Released
Undecided
Unassigned
Mantic
Won't Fix
Undecided
Unassigned
Noble
Fix Released
Undecided
Unassigned

Bug Description

[ Impact ]

graphivz output is incorrect for SVG output when the "size" attribute is set, as it implicitly is in several circumstances.

[ Test Plan ]

* sudo apt install imagemagick graphviz
* Grab fst.dot attached to this bug
* dot -Tpng fst.dot | display png:-
* Observe correct scaling
* dot -Tsvg fst.dot | display svg:-
* Observe incorrect cropping of output
* Enable proposed (https://wiki.ubuntu.com/Testing/EnableProposed)
* Upgrade graphviz to version from proposed
* dot -Tsvg fst.dot | display svg:-
* Ensure output is not cropped

[ Regression Potential ]

Low; the patched applied is from upstream and has been part of the official sources for many years now (Debian and Ubuntu are rather behind on their versions of graphviz, but it looks like this may be corrected in the oracular cycle). Nonetheless, the patch was first applied to our current version, is minimal in nature, and fixes the issue in tests without impacting other output.

There is a very small possibility someone may be relying on the incorrect truncation of SVG output, but this is almost certainly outweighed by the number of users impacted by incorrect truncation of output in, e.g. jupyter notebooks, and by the consistency of having the same (lack of) cropping in different output formats.

[ Original Description ]

SVG output brutally truncates large (and even not so large) graphs when the "size" attribute is set.

This was fixed *two years ago* upstream. Any chance that we might get more recent graphviz packages in Ubuntu at some point? Failing that, it is quite easy to fix this bug.

See https://gitlab.com/graphviz/graphviz/-/issues/1605

The fix is very simple, it seems: https://gitlab.com/graphviz/graphviz/-/commit/a5606d101af1cc949908a6f0bc19caaa4eb31159

Definitely present in 22.04, still present in the Lunar package (graphviz_2.42.2-7build3)

Related branches

Revision history for this message
David Huggins-Daines (dhuggins) wrote :
Revision history for this message
Dave Jones (waveform) wrote :

This looks reasonable, I'll comment further on the associated merge request.

Revision history for this message
David Huggins-Daines (dhuggins) wrote (last edit ):

I hate to be a jerk but it's been over a year, and I even provided a complete fix and patch.

The fix is literally one line. Somewhere in the distant past, 7 years ago I think, somebody made a typo in Graphviz and committed it.

The year is 2024 and we still can't properly view graphs in Jupyter on Debian and Ubuntu because of this.

Please please please.

Revision history for this message
Dave Jones (waveform) wrote :

I quite understand the frustration with something that appears to be abandoned. However, in this case it seems it was actually down to some confusion over the versioning scheme which changed upstream from Debian (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=980900 has the gory details). It also seems that Debian's now got a new watch file and version 11 is now in experimental, so I'm hoping that'll migrate to unstable at some point and we'll pick up a modern version for oracular.

Anyway, back to the fix at hand -- this definitely needs patching in oracular, noble, mantic, and jammy at the very least. Unfortunately your branch appears to have diff-markers embedded in it, so I can't accept it directly, but the patch is trivial enough I can just apply it to the package directly. I'll target this to the affected releases and start the SRU process once this is in oracular.

Changed in graphviz (Ubuntu):
status: New → Confirmed
Changed in graphviz (Ubuntu Jammy):
status: New → Confirmed
Changed in graphviz (Ubuntu Mantic):
status: New → Confirmed
Changed in graphviz (Ubuntu Noble):
status: New → Confirmed
Dave Jones (waveform)
summary: - scaling is wrong in SVG output
+ [SRU] scaling is wrong in SVG output
Revision history for this message
Dave Jones (waveform) wrote :

For SRU purposes, is there any chance I could ask you for a simple reproducer case? It's okay if it uses jupyter, but I'm struggling to come up with one without it and I've run out of time this evening.

Revision history for this message
David Huggins-Daines (dhuggins) wrote (last edit ): Re: [Bug 2015017] Re: [SRU] scaling is wrong in SVG output
  • fst.svg Edit (6.7 KiB, image/svg+xml; name="fst.svg")
  • fst.dot Edit (735 bytes, application/msword-template; name="fst.dot")

Sure, no problem at all! Thanks for the gracious reply, sorry about my
frustration this morning.

You can see the problem with this dotfile by running these commands - the
figure will obviously be clipped (it should have 9 nodes but you can only
see 6):

dot -Tsvg fst.dot
open fst.svg

I've attached the resulting SVG file as well. Note also that if you render
to PDF, PNG, etc, you won't see the clipping.

Revision history for this message
David Huggins-Daines (dhuggins) wrote :
Revision history for this message
Dave Jones (waveform) wrote :

Brilliant, thanks very much for that -- I've upload the ocular version to proposed, and I'll get the SRU uploads done later this morning.

Dave Jones (waveform)
description: updated
Revision history for this message
David Huggins-Daines (dhuggins) wrote :

Thank you, thank you! Appreciate the explanation of the long delay - I think one of the graphviz maintainers said something like "graphviz has gone through multiple different versioning schemes, all of them confusing" and the 2.42.4 release happened right around the point when they stopped explicitly saying "no this one is really actually a release".

I'll sign up for some software anger management in the future :)

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Hi Dave (~waveform),

Thanks for all the work to review/sponsor this fix! (And several others!)
It's really appreciated.

In a comparatively small but relevant note, I'd like to humbly point out though
that some DEP-3 headers seem incorrect (IMHO) in the uploads AFAICT based in [1]
(section `Standard fields`):

+Author: David Huggins-Daines <email address hidden>
+Bug: https://gitlab.com/graphviz/graphviz/-/commit/a5606d101af1cc949908a6f0bc19caaa4eb31159
+Description: Corrects scale factor in SVG output

These headers are totally understandable with the context in this bug,
but it might not always be available when looking at it in the future.

If I understand it correctly:

`Author` isn't David (who proposed the patch; so, credit is certainly due, likely in the changelog, e.g., '[ Name ]' before the entry), but the upstream patch author
> Author or From (optional): This field can be used to record the name and email of the patch author ...

`Bug` should be an upstream bug tracker, not the commit/patch:
> Bug-<Vendor> or Bug (optional): It contains one URL pointing to the related bug ...
> The Bug field is reserved for the bug URL in the upstream bug tracker.
> Those fields can be used multiple times ...
> The vendor name is explicitely encoded in the field name"

`Description` is OK, but looks more like `Subject`, as it's single line.
(BTW, this reads way better than the upstream commit title! :)

I think the correct form should be, starting from `git format-patch` or the `.patch`-suffixed URL [2],
which already gives us:
`From:`, which replaces `Author:` ("Author or From (optional)")
`Subject:`, which replaces `Description:` with a different/unstructured format ("Description or Subject (required)")

And we would add 2 optional fields, but which are highly useful (and more standard, considering most SRUs I've seen so far).

Bug-Ubuntu: http://bugs.launchpad.net/bugs/2015017
Origin: [<upstream|backport|vendor>,] https://gitlab.com/graphviz/graphviz/-/commit/a5606d101af1cc949908a6f0bc19caaa4eb31159

[1] https://dep-team.pages.debian.net/deps/dep3/
[2] https://gitlab.com/graphviz/graphviz/-/commit/a5606d101af1cc949908a6f0bc19caaa4eb31159.patch
[3] https://ubuntu-archive-team.ubuntu.com/proposed-migration/oracular/update_excuses.html#graphviz

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote (last edit ):

To be clear, I don't think these should block the acceptance of this SRU
(I have seen different criteria to accept SRUs regarding DEP-3 headers),
but hopefully might be of some value for future SRUs, if you agree w/ it.

I'll refrain from accepting it for now as oracular-proposed seems to have
some regressions in autopkgtests [3], so let's see how that goes first.

Thanks again!

[3] https://ubuntu-archive-team.ubuntu.com/proposed-migration/oracular/update_excuses.html#graphviz

Dave Jones (waveform)
Changed in graphviz (Ubuntu):
status: Confirmed → Fix Committed
Changed in graphviz (Ubuntu Jammy):
status: Confirmed → In Progress
Changed in graphviz (Ubuntu Mantic):
status: Confirmed → In Progress
Changed in graphviz (Ubuntu Noble):
status: Confirmed → In Progress
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package graphviz - 2.42.2-9ubuntu1

---------------
graphviz (2.42.2-9ubuntu1) oracular; urgency=medium

  [ David Huggins-Daines ]
  * d/p/svg-scaling.patch: Fix scaling in SVG output (LP: #2015017)

 -- Dave Jones <email address hidden> Sat, 11 May 2024 19:31:23 +0100

Changed in graphviz (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Steve Langasek (vorlon) wrote : Please test proposed package

Hello David, or anyone else affected,

Accepted graphviz into noble-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/graphviz/2.42.2-9ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-noble to verification-done-noble. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-noble. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in graphviz (Ubuntu Noble):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-noble
Changed in graphviz (Ubuntu Mantic):
status: In Progress → Fix Committed
tags: added: verification-needed-mantic
Revision history for this message
Steve Langasek (vorlon) wrote :

Hello David, or anyone else affected,

Accepted graphviz into mantic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/graphviz/2.42.2-7ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-mantic to verification-done-mantic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-mantic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in graphviz (Ubuntu Jammy):
status: In Progress → Fix Committed
tags: added: verification-needed-jammy
Revision history for this message
Steve Langasek (vorlon) wrote :

Hello David, or anyone else affected,

Accepted graphviz into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/graphviz/2.42.2-6ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Revision history for this message
David Huggins-Daines (dhuggins) wrote : Re: [Bug 2015017] Please test proposed package

Can confirm that this fixes the problem for me!

Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (graphviz/2.42.2-9ubuntu0.1)

All autopkgtests for the newly accepted graphviz (2.42.2-9ubuntu0.1) for noble have finished running.
The following regressions have been reported in tests triggered by the package:

asymptote/unknown (arm64)
git-big-picture/1.2.2-1 (arm64)
graphviz/unknown (arm64)
libgraphviz-perl/unknown (arm64)
libgraphviz2-perl/unknown (arm64)
objgraph/unknown (arm64)
python-graphviz/unknown (arm64)
python-ruffus/unknown (arm64)
snakemake/unknown (arm64)
sphinx-automodapi/unknown (arm64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/noble/update_excuses.html#graphviz

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (graphviz/2.42.2-7ubuntu0.1)

All autopkgtests for the newly accepted graphviz (2.42.2-7ubuntu0.1) for mantic have finished running.
The following regressions have been reported in tests triggered by the package:

android-platform-art/unknown (arm64)
asymptote/2.86+ds-1 (arm64)
libgraphviz-perl/unknown (arm64)
makefile2graph/unknown (arm64)
python-ruffus/unknown (arm64)
regina-normal/unknown (arm64)
ros-geometry/unknown (arm64)
ros-geometry2/unknown (arm64)
ruby-graphviz/unknown (arm64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/mantic/update_excuses.html#graphviz

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (graphviz/2.42.2-6ubuntu0.1)

All autopkgtests for the newly accepted graphviz (2.42.2-6ubuntu0.1) for jammy have finished running.
The following regressions have been reported in tests triggered by the package:

python-ruffus/unknown (arm64)
ros-geometry/unknown (arm64)
ros-geometry2/0.7.5-9build1 (arm64)
ruby-graphviz/unknown (arm64)
sphinx/unknown (arm64, s390x)
sphinx-automodapi/unknown (s390x)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/jammy/update_excuses.html#graphviz

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Revision history for this message
Jeremy Bícha (jbicha) wrote :

The autopkgtests have passed on retry.

Changed in graphviz (Debian):
status: Unknown → New
Revision history for this message
Jeremy Bícha (jbicha) wrote :

Using Ubuntu 24.04 LTS, I verified the failure test case. Then I installed graphviz 2.42.2-9ubuntu0.1 and verified that the test now passes.

Because Ubuntu 23.10 is scheduled for End of Life today, I will not verify the fix on that release.

tags: added: verification-done-noble
removed: verification-needed-noble
Revision history for this message
Jeremy Bícha (jbicha) wrote :

Using Ubuntu 22.04 LTS, I verified the failure test case. Then I installed graphviz 2.42.2-6ubuntu0.1 and verified that the test now passes.

tags: added: verification-done verification-done-jammy
removed: verification-needed verification-needed-jammy
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package graphviz - 2.42.2-9ubuntu0.1

---------------
graphviz (2.42.2-9ubuntu0.1) noble; urgency=medium

  [ David Huggins-Daines ]
  * d/p/svg-scaling.patch: Fix scaling in SVG output (LP: #2015017)

 -- Dave Jones <email address hidden> Sat, 11 May 2024 19:31:23 +0100

Changed in graphviz (Ubuntu Noble):
status: Fix Committed → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

The verification of the Stable Release Update for graphviz has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package graphviz - 2.42.2-6ubuntu0.1

---------------
graphviz (2.42.2-6ubuntu0.1) jammy; urgency=medium

  [ David Huggins-Daines ]
  * d/p/svg-scaling.patch: Fix scaling in SVG output (LP: #2015017)

 -- Dave Jones <email address hidden> Sat, 11 May 2024 19:31:23 +0100

Changed in graphviz (Ubuntu Jammy):
status: Fix Committed → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote :

Ubuntu 23.10 (Mantic Minotaur) has reached end of life, so this bug will not be fixed for that specific release.

Changed in graphviz (Ubuntu Mantic):
status: Fix Committed → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.