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

How best to construct a ParagraphGraphicFactory that allows for dynamic positioning of Nodes #606

Closed
garybentley opened this issue Oct 6, 2017 · 10 comments

Comments

@garybentley
Copy link

This is a continuation and broadening of the question in: #587

What is the best way to create a ParagraphGraphicFactory that allows for Nodes to be dynamically positioned depending upon the vertical position/offset of a piece of text at a particular index/offset into a paragraph.

For example in the following block of text:

To Sherlock Holmes she is always the woman. I have seldom heard him mention her under any other name. In his eyes she eclipses and predominates the whole of her sex. It was not that he felt any emotion akin to love for Irene Adler. All emotions, and that one particularly, were abhorrent to his cold, precise but admirably balanced mind.

If I wish to add a Node (some kind of visual indicator) against the text "It was not that he felt" in the paragraph graphic to the left I could do:

       area.setParagraphGraphicFactory(new IntFunction<Node> ()
       {

            @Override
            public Node apply (int idx)
            {

                Pane pane = new Pane ();
                pane.setPrefWidth (50);
                pane.setStyle ("-fx-background-color: yellow; -fx-border-width: 1; -fx-border-color: green;");

                try
                {

                    int ppos = area.getAbsolutePosition (idx, 0);
                    Optional<Bounds> posb = area.getCharacterBoundsOnScreen (ppos, ppos);
                    Optional<Bounds> charb = area.getCharacterBoundsOnScreen (ppos + 150, ppos + 151);

                    if (charb.isPresent ())
                    {

                        Label l = new Label ("=>");
                        pane.getChildren ().addAll (l);
                        l.relocate(10, charb.get ().getMinY () - posb.get ().getMinY ());

                    }

                } catch (Exception e) {
                    // Ignore.
                }

                return pane;

            }

        });

This works relatively well for determining the y offset for positioning, the 150 is a nominal value used as an example, however there are a few issues:

  1. On first adding the quoted text via pasting into a GenericStyledArea wrapped in a VirtualizedScrollPane (I just modified the RichText.java example), I get the image below:

example1

It's only when I move paragraph 0 to paragraph 1 by adding a new line above do I get
example2

which is what I would expect.

  1. The call to ParagraphGraphicFactory.apply is only performed once and the Node resized as appropriate in ParagraphBox.layoutChildren. So what is the best way for me to handle the resizing since changing the width of the ParagraphBox may change the y offset of a piece of text? Is there a property or events I can observe to then trigger an update?

  2. The resizing doesn't seem to clip. Here I have expanded the window so that the text is all on a single line, see below (image truncated because of large width). Note the => is still visible below the paragraph bounds.

example3

  1. If I add another new line above then ParagraphGraphicFactory.apply is called again to create another Node for the paragraph graphic. Is this deliberate behavior? I'd like to be able to create a Region once then manage the Nodes within it (i.e. move them around as necessary).

The question really is "am I going about this the right way?" Should I use be using ParagraphGraphicFactory at all or is another technique more appropriate?

@JordanMartinez
Copy link
Contributor

What is the best way to create a ParagraphGraphicFactory that allows for Nodes to be dynamically positioned depending upon the vertical position/offset of a piece of text at a particular index/offset into a paragraph.

So, your goal is something like this?

    | Text text text text text text
=>  | Text <text to indicate> text
    | Text text text text text text
    | Text text text text text text
    | Text text text text text text

And every time you scroll the viewport or insert/delete/replace text, the indicator's position is updated to still point to the same line of text, correct?

      // scrolled up
    | Text text text text text text
    | Text text text text text text
    | [modification] text text text text
=>  | Text <text to indicate> text
    | Text text text text text text

The ParagraphGraphic was added so that one can implement line numbers in a code area. Since these do not change, it renders the graphic once and then relocates the entire graphic (or the Node returned from the IntFunction) on scrolls. In addition, Pane does not clip its children, so your label can appear outside of the pane's bounds.

If I understand your goal, then your basic approach is correct but needs to be improved by relocating the label each time the viewport is changed or the area's content is changed (even if that content doesn't affect the line in question).
The EventStream you'd need to subscribe to is currently private (see here). This would need to be exposed, or you can re-implement it yourself. Then, you could follow this approach:

EventStream<?> viewport = area.getViewportChanges();
EventStream<?> content = area.richTextChanges();
// set up an Event Stream that emits an event any time one of these streams emits one
EventStream<?> renderTrigger = EventStreams.merge(viewport, content);

// subscribe to the trigger stream and store its subscription for later
Subscription sub = renderTrigger.subscribe(ignore -> updateLabelLocation());

EventStreams.changesOf(label.getSceneProperty())
    // set up the following reaction when the 
    // label is added to the scene graph
    .filter(s -> s != null)
    .subscribeForOne(ignore -> 
        // now that it's added
        // set up the following reaction
        EventStreams.ChangesOf(label.getSceneProperty())
        // when the label has been removed from the scene graph
        // due to no longer being needed
        .filter(s -> s == null)
        // unsubscribe sub to prevent memory leaks
        .subscribeForOne(sub.unsubscribe());

@JordanMartinez
Copy link
Contributor

Perhaps, the easiest way to fix this would be to have 2 possible ParagraphGraphicFactory methods: one which renders its node once (current behavior) and one which re-renders its node every time the containing ParagraphBox calls layoutChildren().

@garybentley
Copy link
Author

Yes, that's exactly the goal, to have an indicator move position in the left hand column to "follow" a specific piece of text, or position within the text.

I think your suggestion of a forced relayout of the ParagraphGraphic node would probably be the best way to go. Then I just override layoutChildren and position the Nodes as appropriate. I'll modify my local version of RichTextFX to try this method out and let you know how I get on.

Thanks for the code for stream watching, I will need to watch for text changes to also force a re-positioning, if needed.

@garybentley
Copy link
Author

I've done some tests, it seems that if you just override layoutChildren in your ParagraphGraphic (if you have a Parent Node) then everything works with no intervention needed in ParagraphBox. I don't need to watch for viewport changes either, the ParagraphGraphic.layoutChildren call handles everything.

So:

area.setParagraphGraphicFactory(idx ->
{

    Pane pane = new Pane ()
    {

        @Override
        public void layoutChildren ()
        {

        }

    };

    return pane;

});

However there is one issue, the ParagraphGraphic will often get recreated and of course the paragraph index can change. Thus I have to recreate the Pane (in this case) numerous times but also have to keep references to the created Panes against the paragraph index, then remove the reference when the Pane is removed from the Scene. The trouble here is the overhead of having to manage those Panes and keeping references. Is there a better way to handle the ParagraphGraphics?

Just a couple of other minor things:

  1. Is there a way to have a paragraph graphic fill the entire viewport when there is no text in the text area? For my purposes the user will be faced with a blank "sheet" that they will write on and I would like to have the left hand margin/column always visible.

  2. If you call GenericStyledArea.getCharacterBoundsOnScreen with the same index for from and to you get back the start position of the parent paragraph, regardless of what from you use.

i.e. area.getCharacterBoundsOnScreen (0, 0)

This trapped me for a while. It would be good to have an exception thrown if the same value is passed in or a -1 returned to indicate the error.

  1. What is your policy on adding to the wiki? I have created a basic position tracking mechanism based on watching the richTextChanges stream and would like to share it for others to use (or laugh at, their choice).

@garybentley
Copy link
Author

Actually I retract my statement about ParagraphBox.layoutChildren not needing to call requestLayout on the ParagraphGraphic. If you don't request the layout (as shown below) then there are occasions when the graphic won't layout correctly and positioning can go awry.

Adding:

        graphic.ifPresent (g ->
        {

            if (g instanceof javafx.scene.Parent)
            {

                ((javafx.scene.Parent) g).requestLayout ();

            }

        });

At line 231 of ParagraphBox solves the problem, although I'm unhappy about the nasty cast.

@JordanMartinez
Copy link
Contributor

Is there a way to have a paragraph graphic fill the entire viewport when there is no text in the text area? For my purposes the user will be faced with a blank "sheet" that they will write on and I would like to have the left hand margin/column always visible.

You could create your own Region implements Virtualized and layout a margin node to the left of the area node. Then, you'd need to map the estimatedScrollXProperty and related scrolling properties to incorporate the margin node's width/height

If you call GenericStyledArea.getCharacterBoundsOnScreen with the same index for fromand toyou get back the start position of the parent paragraph, regardless of what from you use.

I'm not sure I understand what you mean by "parent paragraph." Could you clarify that?

What is your policy on adding to the wiki? I have created a basic position tracking mechanism based on watching the richTextChanges stream and would like to share it for others to use (or laugh at, their choice).

We don't really have a policy. If you feel that it's useful, write and publish a page on what you have in mind and then open an issue about it so we can further discuss it. It might make more sense to discuss it in an issue first in case you have misunderstandings about some of the inner workings of things, but I think writing long documents in the Wiki page editor is easier than doing so in a regular issue.

@JordanMartinez
Copy link
Contributor

Actually I retract my statement about ParagraphBox.layoutChildren not needing to call requestLayout on the ParagraphGraphic. If you don't request the layout (as shown below) then there are occasions when the graphic won't layout correctly and positioning can go awry.

Does this mean we should use the idea I mentioned before of a lazy and strict ParagraphGraphic? We could get around the cast by changing the type from IntFunction<Node> to IntFunction<Either<Parent<Either<Control, Group>> and add convenience methods to wrap the given paragraph graphic into the correct Either object. You wouldn't need to cast anything with that approach.

@garybentley
Copy link
Author

I tried your suggestion of a separate Region and that worked beautifully. You don't need to implement Virtualized, just listen for the estimatedScrollYProperty and boundsInLocalProperty to watch for changes in scrolling position and viewport size.

A call such as the one below handles things:

vsPane.estimatedScrollYProperty ().addListener ((ev, oldv, newv) -> Platform.runLater (() -> myRegion.requestLayout ()));

I found that the Platform.runLater was required to update the region on a future pulse otherwise the markers in the region would lag behind the current scroll position or viewport size. The runLater ensures that they look synchronized to the user. Then in the layoutChildren of the Region move the markers to the appropriate positions for the currently visible text.

I'll add a full example as a new issue so that people can have look and use the method for tracking and highlighting text positions in a "margin" (or elsewhere).

I think the change to ParagraphBox.layoutChildren is still required to ensure that the paragraph graphic updates and gets a chance to reflect the associated paragraph. And anything to get rid of casting! I won't be using ParagraphGraphics but I can see them being very useful for many applications.

Thank you for all your suggestions and help on this issue. Now that I can track positions in the text and display the markers the GenericStyledArea and VirtualizedScrollPane will finally allow me to update my application to JavaFX, so thank you for all the hard work you've done on this project.

@JordanMartinez
Copy link
Contributor

Glad you found a solution!

I'll add a full example as a new issue so that people can have look and use the method for tracking and highlighting text positions in a "margin" (or elsewhere).

Mind submitting a PR to the demo package?

@garybentley
Copy link
Author

Will do, I have been working on an example for the past couple of days. However the demo will require GenericStyledArea.getRangeBoundsOnScreen to be public to work. I have generalized getCharacterBoundsOnScreen to handle new line positions (which are valid positions for users to want to add things to even if a character is not visible, i.e. you might click at the end of a paragraph to indicate something). getRangeBoundsOnScreen depends on access to the virtualFlow which is private in GenericStyledArea and no public accessor method available for it so I can't just add that as well.

If I do a pull request how do we handle that? Should I not worry about it and just do the request?

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

No branches or pull requests

2 participants