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

Support wavy underlines (issue #1131) #1132

Merged
merged 4 commits into from
Jul 21, 2022
Merged

Support wavy underlines (issue #1131) #1132

merged 4 commits into from
Jul 21, 2022

Conversation

shoaniki
Copy link
Contributor

Basic implementation of wavy underlines as described in #1131.

Adds two properties -- "wave" to control the radius of arc used to draw wavy lines, and "offset" to adjust the distance between text and underline, since it can be necessary to increase this for legibility when a wavy underline is in use. Probably better names are possible.

These are used in TextFlowExt#getUnderlineShape to modify the shape produced. If no radius is specified then a line is drawn as previously; otherwise a series of arcs is drawn, which replicates the appearance of the wavy underlines in common programs such as Microsoft Word.

I have only implemented the wavy-line shape in the Java 9+ version of the class because I can see there's an open question whether it's even worth supporting Java 8 any more (and my opinion FWIW is "no").

I haven't added any tests – if they're required then I would appreciate advice on how to test a purely visual change like this.

@Jugen
Copy link
Collaborator

Jugen commented Jul 19, 2022

This is great, thanks a stack for this PR !

Copy link
Collaborator

@Jugen Jugen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please for clarity, change all wave variables to waveRadius please.

richtextfx/src/main/java/org/fxmisc/richtext/TextExt.java Outdated Show resolved Hide resolved
@Jugen
Copy link
Collaborator

Jugen commented Jul 19, 2022

Okay, so I've played around with the code you've submitted and propose the following changes:

  1. I noticed that there's a relationship between the wave radius and the offset, in that the offset needs to be increased as the radius gets larger. I think providing a reasonable default will help developers to not have to figure out another underline option if they don't need to. Suggested change to ParagraphText.UnderlineAttributes:
Number waveNumber = text.getUnderlineWave();
waveRadius = waveNumber == null ? 0 : waveNumber.doubleValue();
Number offsetNumber = text.getUnderlineOffset();
offset = offsetNumber == null ? waveRadius*0.5 : offsetNumber.doubleValue();
// The larger the radius the bigger the offset needs to be, so
// a reasonable default is provided if no offset is specified.
  1. The range of useful wave radii is quite limited and with larger radii the underline is rather bumpy and not wavy. If however the X radius of the arcs is stretched out slightly then the range of useful radii increases and the underline is smoothed/stretched out as well. Suggested change to TextFlowExt.getUnderlineShape:
for ( int ele = 2; ele < shape.length; ele += 5 )
{
    LineTo bl = (LineTo) shape[ele+1];
    LineTo br = (LineTo) shape[ele];

    double y = snapSizeY( br.getY() + offset - 2.5 );
    double leftx = snapSizeX( bl.getX() );

    if (waveRadius <= 0) {
        result.add(new MoveTo( leftx, y ));
        result.add(new LineTo( snapSizeX( br.getX() ), y ));
    }
    else {
        // For larger wave radii increase the X radius to stretch out the wave.
        double radiusX = waveRadius > 1 ? waveRadius * 1.25 : waveRadius;
        double rightx = snapSizeX( br.getX() - radiusX ); // Prevent overshooting.
        result.add(new MoveTo( leftx, y ));
        boolean sweep = true;
        while ( leftx < rightx ) {
            leftx += waveRadius * 2;
            // Note, had to remove code here otherwise underline ended in strange tail ?
            result.add(new ArcTo( radiusX, waveRadius, 0.0, leftx, y, false, sweep ));
            sweep = !sweep;
        }
    }
}

I would appreciate it if you could try out the above changes and see how it affects what you want.
If you have any concerns, differences, or recommendations please comment.

@shoaniki
Copy link
Contributor Author

Thank you for giving it a try! I agree on both counts -- the shallower waves look much nicer! -- and have pushed an update accordingly.

The only difference is that I have restored a version of the endpoint computation code, fixed to remove the assumption that the arc is defined by a circle. I think it's necessary to clip the path this way, because otherwise larger radii will always either overflow or underflow. I appreciate the code is a bit dense, but I wasn't able to find a more idiomatic way to do the clipping (which surprised me a bit -- I was expecting JavaFX to have that kind of feature, did I miss something?)

Copy link
Collaborator

@Jugen Jugen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the effort you're putting into this !

@Jugen Jugen merged commit 9ce8c84 into FXMisc:master Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants