tesis-dynaware / graph-editor

Eclipse Public License 1.0
132 stars 42 forks source link

Make the grid color customizable via CSS #1

Closed eckig closed 9 years ago

eckig commented 9 years ago

Not much more to say: Please add a style-class to the GraphEditorGrid and/or the grid lines so third party developers can change that color, too.

Otherwise it is really odd that we may change every color with CSS exluding the grid color.

rmfisher commented 9 years ago

I agree this is odd. Initially I styled the grid via CSS just like everything else, but the performance was noticeably worse. I'll take another look at it.

eckig commented 9 years ago

If you like I can experiment with this, too. One thing to speed this up would be to paint the grid only to viewport boundaries. If I resume this correctly, currently the whole grid is painted?

rmfisher commented 9 years ago

Currently all grid-lines are added to the scene graph. Only the lines in the viewport are rendered, because everything outside the visible area is clipped.

Adding and removing nodes from the scene graph is an expensive operation. I don't think you would want to add and remove grid-lines dynamically, as it would make the 'panning' motion laggier.

I looked at setting the grid line color via CSS. Unfortunately if you resize the view, you can see the grid lines flicker as the CSS style takes a small amount of time to be applied.

I can easily provide an API method such as:

graphEditor.getProperties().setGridColor(...)

This would then override the default color if it was not null. Would this be ok? Otherwise you are welcome to play around and see if you can find a more elegant solution.

eckig commented 9 years ago

I tried 100000 x 100000 pixel size grid: https://gist.github.com/eckig/97cdb42900c2e07e9fd0 Works just nice.

What I meant with a smaller grid is the following: Draw the grid only with the size of the clipping rectangle and add it as a layer behind the GraphEditorContainer.

Currently the Grid is located as a background layer in the GraphEditorView where there is no clipping functionality. You could refactor the GraphEditorView into a new base class (BasicGraphEditorView) containing all of the GraphEditorView functionality excluding the Grid and make the GraphEditorView subclass BasicGraphEditorView, just adding the Grid. Now the Grid can be added to GraphEditorContainer as a background layer where it is only as large as the clipping rectangle.

Talking about the GraphEditorContainer: It would be nice if you could expose the MiniMaps visible property, so we can bind it e.g. to a ToggleButtons selected property ;-)

rmfisher commented 9 years ago

If you added the grid as a background layer of the container itself, how would you ensure that the grid lines were positioned correctly? As the panning window moves across the view, the grid-lines should stay put.

The minimap visible property should be exposed, good point. I will add this soon.

eckig commented 9 years ago

There are two ways to do so:

Grid line positioning is fixed / ignored - easy.

Or you can draw the grid a little bit larger than the clipping rectangle (at least the grid spacing). So you can move the grid around imitating the original behavior.

rmfisher commented 9 years ago

If the grid position is fixed relative to the position of the panning window, the grid lines will not be in sync with the positions that the nodes and joints are snapping to in the view. I would rather not implement it like this.

Your second suggestion could work nicely, again if you want to submit a pull request I will try to integrate it.

eckig commented 9 years ago

Ok, I think I have a solution:

When the parent is resized the grid will draw twice at a minimum, when width and height are set. If you trigger draw off layout you will get at most one draw per frame. Next step was to convert the whole thing to a Canvas to avoid too many nodes in the SceneGraph. Next step was to add CSS styleable properties for the spacing and color of the grid, the result is the ResizableGrid: https://gist.github.com/eckig/176b7c2a10048bb71f43

The performance is pretty good and we may even boost that further by improving calls to g.clearRect(0, 0, width, height); As we do not have to clear the whole canvas, only the part which has changed?

And to the width and height of this ResizableGrid: Please remove

setMinSize(1000, 1000);
setPrefSize(1000, 1000);

from the Constructor and bind the size to the parent (StackPane) size. This way the Grid gets repainted accordingly with layoutChildren.

Now you can just write the following in CSS:

.graph-grid {
  -graph-grid-color: gray;
  -graph-grid-spacing: 25;
}
rmfisher commented 9 years ago

The styleable properties are very nice, as is the layoutChildren approach. I am not 100% sure about the Canvas, I think when you zoom via a scale transform the grid lines will look pixellated.

I will try to incorporate some of these ideas. Unfortunately I'm very busy with another project and it may take a few weeks.

eckig commented 9 years ago

Well that was a first draft, but I am glad that you like it. If you are okay with that, I can try scaling tomorrow and if that works or I get a workaround for that, I can create a pull request?