Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distorted ABC (SVG) score snippets #399

Closed
gwyndaf opened this issue Jul 27, 2024 · 21 comments
Closed

Distorted ABC (SVG) score snippets #399

gwyndaf opened this issue Jul 27, 2024 · 21 comments
Labels

Comments

@gwyndaf
Copy link

gwyndaf commented Jul 27, 2024

I've a few songs in my collection for which ABC score snippets have become distorted (vertically stretched) with 6.050_096 (possibly earlier, but not something I was checking).

Previously:
Screenshot from 2024-07-27 22-58-54
Now:
Screenshot from 2024-07-27 22-59-07

From source:

{start_of_abc-keyboard}
%%singleline 1
%%maxshrink 0.68
X:1
K:A octave=-1
M:4/4
L:1/4
|: "A"c3 B | "D"d3 c/4B/2A/4 |[1 "A"B3/4c/4- c- c3/4"A/C#"[ca]/4 [ca]3/4"E/B"[Bg]/4 | [Bg]3/4"D/A"[Af]/4 [Af]12/4 :|] [2 "A"B3/4c/4- c2 "E/G#"[B,EB] | "D/F#"[A,DA]3/2 "A/E" [A,-C-A-]/2 [A,CA]2  |]

{end_of_abc}

I have several score snippets now exhibiting the same behaviour, and the common factor seems to be that all are quite long, so I have to tweak ABC %% settings or ChordPro scale to fit a single page width and achieve consistent staff height across all songs. Short scores (less than a page width) don't seem to be affected.

debug.abc output is

{
    abclib  => '/media/data2/Shared/Music/Technology/ChordPro/git/chordpro/lib/ChordPro/res/abc',
    handler => 'abc2svg',
    info    => 'QuickJS_XS (ABC2SVG version v1.22.18_9b12853f66 of 2024-07-08)',
    method  => 'QuickJS_XS',
    version => 'ABC2SVG version v1.22.18_9b12853f66 of 2024-07-08'
}
+ QuickJS_XS[/media/data2/Shared/Music/Technology/ChordPro/git/chordpro/lib/ChordPro/res/abc/abc2svg] /tmp/kHlkzzXmIy/tmp1.abc
 QuickJS_XS using /media/data2/Shared/Music/Technology/ChordPro/git/chordpro/lib/ChordPro/res/abc/cmd.js+hooks
ABC: staffbase = -52.58110046386719

At a quick check, Firefox seems to display the temporary SVG with correct (unstretched) proportions. This is it: tmp1

I recall previous discussion that ChordPro default scaling for ABC SVG is to fit page width, so I'm wondering if a recent change has affected that scaling so it's only scaling the width, and not the height, of the inserted SVG?

@sciurius
Copy link
Collaborator

It's a bug introduced recently when I added independent h/v scaling. I'll fix it.

@sciurius sciurius added the bug label Jul 29, 2024
ChordPro pushed a commit that referenced this issue Jul 29, 2024
@sciurius
Copy link
Collaborator

Fixed in 6.050_099.

@gwyndaf
Copy link
Author

gwyndaf commented Jul 29, 2024

Great. That looks good for regular {start_of_abc}

However, in <img>, the scale attribute seems to be troublesome.

For instance:

{title: ABC image scaling}
{start_of_abc: riff 1}
id=riff1
scale=0.76
%%singleline 1
%%maxshrink 0.4
X:1
K:C
L:1/4
"Dm7"z2 "G7"z3/4 G/4 B/4_B/4A/4_A/4 | "Cmaj7"G2 z2 |

{end_of_abc}

{comment: <span rise=370%>riff 1:</span> <img id="riff1" scale=76% />}

Results in (default config):
Screenshot from 2024-07-29 12-43-37

If I remove the scale setting from the <img> tag, score returns (but larger than I need of course):
Screenshot from 2024-07-29 12-45-49

@sciurius
Copy link
Collaborator

This is what I get from 6.050 and dev (after removing riff 1 from start_of_abc).
6050.pdf
dev.pdf

@gwyndaf
Copy link
Author

gwyndaf commented Jul 29, 2024

Oh, that's odd. I'd already tried removing the riff 1 label and adding quotation marks as id="riff1" but they didn't change anything for me.

No problem with 6.050, only dev version (including 6.050_100). Same behaviour using chord or src image in <img>, which seems to rule out ABC causes. Only removing the scale attribute seems to make it work as expected.

Seems like the difference might be my system. Does anything look outdated in my --about results?

Run-time information:
  ChordPro core          6.050_100  (Unsupported development snapshot)
  Perl                   v5.38.2    (/usr/bin/perl)
  Resource path          ~/.config/chordpro
                         /media/data2/Shared/Music/Technology/ChordPro/git/chordpro/lib/ChordPro/res
  ABC support            QuickJS_XS (ABC2SVG version v1.22.18_9b12853f66 of 2024-07-08)

Modules and libraries:
  Storable               3.32      
  Object::Pad            0.809     
  Text::Layout           0.037     
  File::LoadLines        1.046     
  PDF::API2              2.047     
  SVGPDF                 0.087     
  Font::TTF              1.06      
  JavaScript::QuickJS    0.21      
  JSON::Relaxed          0.096     
  JSON::XS               4.03   

@sciurius
Copy link
Collaborator

My guess is that you have a small(er) page width (columns?). This causes the comment line

<span rise=370%>riff 1:</span> <img id="riff1" scale=76% />

to wrap into

<span rise=370%>riff 1:</span> <img id="riff1 scale=76%
"/>

Now the <img> is no longer recognized.

Peculiar, in my test it is the scaled image that is ok and the non-scaled that goes wrong.

issue399.cho.txt

I really really REALLY must revise the wrapping algorithms.
issue399.pdf

@gwyndaf
Copy link
Author

gwyndaf commented Jul 30, 2024

I don't think it's wrapping, although that was my first thought when I saw /> wrapped to the next line quite consistently. This is all using default -X config, so no columns or other restriction on page width.

I've stripped the test sample down to the <img> tag alone: image_scaling.cho.txt, and the same general behaviour persists: image_scaling_dev.pdf

However, two interesting observations arise (with dev):

  1. If I use width and height attributes instead of scale, asset is displayed, but it's much smaller than I'd expect for a value of 76%.

  2. With width and height values, scaling seems different for width and height, so it's squashed horizontally. This behaviour also applies with release 6.050: image_scaling_release.pdf

Possibly I'm not understanding width and height syntax or usage, but maybe this offers some further insight.

I'll continue experimenting with this, though, because differences between your and my experiences suggest there's maybe something system-specific about it.

@gwyndaf
Copy link
Author

gwyndaf commented Jul 30, 2024

On second thoughts (re-reading documentation), I think maybe width and height percentages might be relative to the available page area, not the original asset, which would explain apparent distortion.

However, the very small image resulting from 76% width and height settings suggests that ChordPro maybe thinks the available area is much smaller than it actually is. Perhaps this also explains apparent wrapping issues.

Not sure if it's directly related, but a similar area of functionality, so I explored the scale, width and height settings for {image}, and get some interesting results:

image_scaling.cho.txt
image_scaling.cho.pdf

  1. The first instance of {image} (with no scaling value) inserts the asset at 76% scaling. Maybe this suggests the scale value is being carried over from scaling for an earlier usage? I suppose I expect that each usage of the asset would be independent of any others.

  2. width and height settings give very different results (PDF pp 2-3) from same values in <img> tag, although seems maybe closer to 76% of page width and height than the <img> instance.

@sciurius
Copy link
Collaborator

I think maybe width and height percentages might be relative to the available page area, not the original asset,

It is even documented for {image}:

# width=width

Specifies the desired width of the image in typographic points (1/72 inch or 0.3528 mm), or a percentage of the available width.

In the case of <img>, it's a percentage of the current font size (hence the very small image). This is not really usefull. I think it is better to disallow percentages for width and height in <img> until we find a better meaning.

The first instance of {image} (with no scaling value) inserts the asset at 76% scaling.

There is a scale=0.76 in the abc section itself. The asset is already scaled.

The problem with the /> wrapped to the next line seems to be gone in Text::Layout 0.037_004.

Does this explain most (all?) of the anomalities?

@gwyndaf
Copy link
Author

gwyndaf commented Jul 30, 2024

OK, that explains the small image resulting from <img ... width ... height .../>, and I agree that percentage values could be confusing. Although I can see the logic, it wouldn't occur to me that width and height percentages relate to character size, rather than to page/column space when those same attribute names are used in {image}.

I suppose there's a potential use case for inserting small graphical symbols as part of text, where percentage values are more adaptable than point sizes, e.g. repurposing the same song source for both regular and 'large type' versions. Maybe it just needs a documentation note to make clear that it's a percentage of character size (not available page/column space)?

Sorry: my failure to notice that I'd already set scale=0.76 in {start_of_abc} so it makes sense that it gets inserted at that scale by default.

However, I notice that principle seems true only for {image id="riff1"}, while <img id="riff1" /> (no scale value) seems to insert it at 100% (not 76%), which seems inconsistent. IIRC that's probably why I originally added scale to <img> even though it was already there in {start_of_abc}.

Yes, if wrapping of /> is affecting the scale function in <img> (for id, src, or chord`), that seems to explain the behaviour. If Text::Layout 0.037_004 is a development version, I'll probably need to wait for the next release version via CPAN.

@sciurius
Copy link
Collaborator

I suppose there's a potential use case for inserting small graphical symbols as part of text, where percentage values are more adaptable than point sizes, e.g. repurposing the same song source for both regular and 'large type' versions. Maybe it just needs a documentation note to make clear that it's a percentage of character size (not available page/column space)?

For the time being I will disable recognition of percentages in <img> width and height (and document it).
If you want textsize-dependent dimensions you can use ex and em. 76% is currently the same as 0.76em.

It is easy to add the development version of Text::Layout:

git checkout https://github.com/sciurius/perl-Text-Layout.git
cd perl-Text-Layout/lib
cp -rp Text WHEREVERYOURCHORDPROIS/lib/ChordPro/lib

seems true only for {image id="riff1"}, while

ChordPro 6.050_102 + Text::Layout 0.037_006 should do it better.

@gwyndaf
Copy link
Author

gwyndaf commented Jul 31, 2024

Thanks Johan, Text::Layout (and ChordPro) both updated successfully, which seems to address the main issue of scale functioning in <img>. Test and production samples now behaving nicely :)

One very minor observation. If the asset is scaled when it's defined (in {start_of_abc}), any subsequent scaling seems to behave differently according to whether it's used with {image} and <img>.

image_scaling.cho.txt gives image_scaling.cho.pdf

With <img>, scale seems applied in addition to the asset's original scaling, but with {image}, scale ignores original scaling - probably applying scale directly to the temporary image (i.e. 100%).

It's only a minor inconsistency, and shouldn't really arise (much). I can't think of a good case for needing to scale an asset both at its original definition and when it's used/inserted (although each of those is useful in itself).

My only reason for reiterating scale previously was as a workaround, i.e. using the asset with <img> didn't retain its original scaling, so it needed restating. Now that's fixed, I can't see a need to use scale twice.

@sciurius
Copy link
Collaborator

Good observation. I will address that later when it's less hot in here.

@sciurius
Copy link
Collaborator

sciurius commented Aug 1, 2024

It turns out that the way I tried to support this screws up other things.

Since it only applies to ABC images, wouldn't it be better to use %%pagescale 0.76 instead?

{start_of_abc}
id=riff1
%%pagescale 0.76
%%singleline 1
%%maxshrink 0.4
X:1
K:C
L:1/4
"Dm7"z2 "G7"z3/4 G/4 B/4_B/4A/4_A/4 | "Cmaj7"G2 z2 |

{end_of_abc}

Later, when I have cleaned up the image handling, I can add official support for scale and deprecate the use of property settings inside the start/end.

{start_of_abc id="riff1" scale="76%"}
%%singleline 1
%%maxshrink 0.4
X:1
K:C
L:1/4
"Dm7"z2 "G7"z3/4 G/4 B/4_B/4A/4_A/4 | "Cmaj7"G2 z2 |

{end_of_abc}

Spread images are screwed up as well, no need to investigate/report.

@gwyndaf
Copy link
Author

gwyndaf commented Aug 1, 2024

Ah, ok. For clarity, do you mean support for consistent scale behaviour between <img> and {image}?

That's quite a marginal issue, so maybe simplest to leave it inconsistent for the time being.

A few reasons for not relying on %%pagescale as a short-term fix:

  1. As a guiding principle, it seems sensible to prioritise clear and reliable functioning of the basic {start_of_abc} functionality, and not compromise that to improve more advanced/niche usage like creating image assets. I guess my rough indicator is "How many users would this change benefit vs. confuse/frustrate?".

  2. (Novice) users of ChordPro may not be familiar with using ABC at a more advanced settings level, so it might not be appropriate asking them to use %%pagescale (an ABC function) to achieve a ChordPro output result.

  3. IIRC, default scaling behaviour in simple {start_of_abc} usage was changed, so large ABC snippets now get inserted at page width by default, and subsequent scale is relative to that size. This still seems most sensible as it avoids trial and error to get a good page fit for larger scores.
    In such cases, I don't think %%pagescale%% has much (or predictable) effect on final sizing. I generally use that (and/or %%maxshrink) to tweak ABC fitting of notes and splitting of multiple stave lines within the generated score (e.g. to match score lines to chord grid layout for a song). It's a fairly rough approach, but seems effective.
    I'll then often use scale as well, to then ensure that score snippets have consistent size (stave height) across all songs.

If you expect to look at this in the longer term anyway, I think maybe better that short-term solution is a bit of inconsistency for very niche cases, rather than changing the main (non-asset) {start_of_abc} behaviour.

ChordPro pushed a commit that referenced this issue Aug 1, 2024
@sciurius
Copy link
Collaborator

sciurius commented Aug 6, 2024

My "I think I fixed it, please confirm" mail got lost ☹.
issue399.zip

Note that the spread images have not been addressed yet.

@gwyndaf
Copy link
Author

gwyndaf commented Aug 9, 2024

Sorry for slow response.

Using 6.050_107, I can confirm that {image} and <img> scaling behaviour now seem consistent, and applied in addition to any scaling applied in {start_of_abc} section.

With your samples, I now get the same output results, and output from my own production songs looks as expected.

@gwyndaf
Copy link
Author

gwyndaf commented Aug 9, 2024

However, one possible new glitch crops up.

With a couple of ABC scores placed using

{comment: Intro and C riff:  <img id="riff1" dy=-235% />        F riff:  <img id="riff2" dy=-235% />}

I'm now getting errors (and warning image on PDF page):

Invalid img attribute: "dy=-235%" (percentage not allowed)

Oddly, the message occurs 4 times, even though 2 dy values are given.

Seems to work nicely if dy is given in point units, though. That's fine for me, as I've no particular need to use percentages here.

Perhaps this is like removing percentage height/width in <img> discussed already, as percentage of character size might be counter-intuitive.

If this isn't intended, it's maybe a small issue. If intended, might need a slight documentation update: beta docs still mention percentages as valid for <img>, for both height/width and offset.

@sciurius
Copy link
Collaborator

The message is intentional for the reasons that you already noticed. I thouch I fixed all occurrences in the docs but I'll recheck.

That the message occurs twice is because the content is evaluated twice. First to obtain its dimensions to see if it fits, and then again for the actual placement. That is one of the things I want to straighten out in a future version.

@gwyndaf
Copy link
Author

gwyndaf commented Aug 11, 2024

It's conceivable that my browser still had the old page cached. I've lost the habit of refreshing manually.

Doc page looks clearer now, but a redundant close parenthesis ) at the end of the dx=NNN entry. Minor, but could confuse, given the preceding "(in points)" parentheses.

@sciurius
Copy link
Collaborator

Good catch. Thanks.

@gwyndaf gwyndaf closed this as completed Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants