Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Added generic graph drawing loop; isolated point drawing of graph style 'Bar' #2208

Merged
merged 4 commits into from
Oct 22, 2016

Conversation

wimrijnders
Copy link
Contributor

Redo of PR #2205, which I messed up in a very bad way.

I hope this is good now.

All graph drawing routines follow the following structure:

Graph3d.prototype._redrawGraph = function() {
  var ctx = this._getContext()

  if (this.dataPoints === undefined || this.dataPoints.length <= 0)
    return; // TODO: throw exception?

  this._calcTranslations(this.dataPoints);

  for (i = 0; i < this.dataPoints.length; i++) {
    point = this.dataPoints[i];

    // Draw data point here
  }
};

It is possible to consolidate this loop and move graph-style specific point drawing
to separate routines. This PR shows how that will look like; I've done only a single
graph style (Bar) to illustrate this.

Changes:

  • made generic routine _redrawDataGraph(), usable for all graph styles.
  • made specific routine _redrawBarGraphPoint(), which draws a single point for style Bar
  • as an extra, isolated the bar drawing code in _redrawBar(), so it can be reused.
  • in method redraw(), pass _redrawBarGraphPoint as a reference to _redrawDataGraph()

The idea is:

  • to make the drawing loop generic for all graph styles
  • isolate the drawing of points for specific graph style in separate methods.
  • do this for all present graph styles.

Notes

1. This is a very important update in regard to handling multiple graphs in a view.
The reason is that the elements of the multiple graphs will need to be drawn together at the same time, so that the overlap of the various elements are correct in the view.

2. I would much rather prefer to use graph-style specific classes here, instead of passing
a point-drawing method.
However, I fully appreciate that implementing that in one go will be a 'Big Bang' change
which is totally unoverseeable for reviewing. For this reason, I am offering the changes in smaller,
bite-size chunks. The classes will have to wait.

@wimrijnders
Copy link
Contributor Author

The last commit adds a useful change with the determination of the point drawing method to use.

I added it because git refused me to make an empty commit.

Copy link
Member

@mojoaxel mojoaxel left a comment

Choose a reason for hiding this comment

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

Keep up the work!

@mojoaxel
Copy link
Member

@wimrijnders Have a look at git --amend.

@wimrijnders
Copy link
Contributor Author

wimrijnders commented Oct 22, 2016

@mojoaxel thanks I will. Actually I'll do it now.

OK did some google research. I'm not sure how it would have helped me with my struggles in #2205 , but I have never used --amend so it was useful to learn.

@yotamberk yotamberk merged commit 105356b into almende:develop Oct 22, 2016
@wimrijnders wimrijnders deleted the PR19 branch October 22, 2016 17:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants