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

Use griddle for the bar graph? #31

Closed
jessegreenberg opened this issue Oct 24, 2018 · 12 comments
Closed

Use griddle for the bar graph? #31

jessegreenberg opened this issue Oct 24, 2018 · 12 comments
Assignees

Comments

@jessegreenberg
Copy link
Contributor

It would be nice if the bar graph could use griddle, the common repo for graphs. Common code may already have a solution for the challenges I am facing in #25 with respect to stroking the bars and transparent rectangles.

The challenge is that ESP has a separate layer for bar graph foreground/background, because the foreground must use webgl for performance while the background must not. But maybe I can still use portions of griddle.

@jessegreenberg jessegreenberg self-assigned this Oct 24, 2018
@jessegreenberg
Copy link
Contributor Author

For these reasons, I don't think we can use BarChartNode, but maybe we can still use BarNode for the bars

@jessegreenberg
Copy link
Contributor Author

I don't think we can use griddle with the current setup. Here is one potential way we could get this to work with the setup in ESP. But here is one way, changing the structure slightly.

  • BarGraphBackground contains the axis and buttons and arrows, added behind the EnergySkateParkScreenView webgl layer.
  • BarGraphForeground contains the bars, added on top of the foreground in the EnergySkateParkScreenView webgl layer.
  • BarGraphLabelTopLayer contains the labels, added on top of the foreground and on top of the webgl layer in EnergySkateParkScreenView.

I am hesitant to use griddle still, because BarNode has other nodes that we do not want in the webgl layer of EnergySkateParkScreenView, like the arrow nodes.

@jessegreenberg
Copy link
Contributor Author

Whether or not we can use griddle in this sim depends on phetsims/griddle#32.

@jessegreenberg
Copy link
Contributor Author

phetsims/griddle#32 was closed, I decided that it wasn't beneficial to griddle to change composite types for use in this sim. So using griddle is now dependent on #42, if we no longer use WebGL, then we can use griddle as is.

@jessegreenberg
Copy link
Contributor Author

Now that we have a decision in #42, we can try to use griddle again. That will be done next as part of #66

@jessegreenberg
Copy link
Contributor Author

The other portion that is unsuported by griddle is

        // Floor and protect against duplicates.
        // Make sure that nonzero values
        // For thermal and total energy, make sure they are big enough to be visible, see #307
        // For kinetic and potential, they must go to zero at the endpoints to reach learning goals like
        //   "The kinetic energy is zero at the top of the trajectory (turning point)
        var smallValueThreshold = showSmallValuesAsZero ? 1 : 1E-6;
        var answer = absResult > 1 ? Math.floor( result ) :
                     absResult < smallValueThreshold ? 0 :
                     1;

@jessegreenberg
Copy link
Contributor Author

@samreid @arouinfar here is what published sim looks like when skater is at track endpoint:
image

Compared to master using griddle:
image

Setting borderWidth to 0 in griddle/BarNode it looks like
image

Though it was more difficult for me to reach an invisible KE bar (I had to use step button), and bar strokes were requested in #25.

Is this sufficient or should we add support to griddle to hide bars if values are too small. Maybe more generally we could add support to griddle to hide bars for any reason.

@samreid
Copy link
Member

samreid commented Apr 25, 2019

It seemed important during design that those bars would disappear completely. I like your idea to investigate doing that in griddle.

jessegreenberg added a commit to phetsims/energy-skate-park-basics that referenced this issue Apr 26, 2019
@jessegreenberg
Copy link
Contributor Author

OK, thanks! Ill take it over there.

@jessegreenberg
Copy link
Contributor Author

I made phetsims/griddle#35

@arouinfar
Copy link
Contributor

It seemed important during design that those bars would disappear completely. I like your idea to investigate doing that in griddle.

Agreed! Seems like a good addition to griddle.

@jessegreenberg
Copy link
Contributor Author

jessegreenberg commented May 22, 2019

Griddle is now being used for the bar graph in this sim. BarGraphForeground and BarGraphBackground have been collapsed into EnergyBarGraph. At this point I think BarGraphForeground and BarGraphBackground can be removed for #66, and this issue can be closed.

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

3 participants