Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add reference code for Lights, Camera #115

Merged
merged 20 commits into from
Jun 13, 2017
Merged

Conversation

gaocegege
Copy link
Member

ref #80

Signed-off-by: Ce Gao ce.gao@outlook.com

Signed-off-by: Ce Gao <ce.gao@outlook.com>
@gaocegege gaocegege changed the title Add reference code for Lights, Camera WIP: Add reference code for Lights, Camera Jun 12, 2017
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Copy link
Member

@jeremydouglass jeremydouglass left a comment

Choose a reason for hiding this comment

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

Excellent.

Things from this commit that could also be applied to all other examples/references in master:

  1. 0-indexed filenames/paths
  2. PI <- pi

@@ -0,0 +1,18 @@
P3D <- "processing.opengl.PGraphics3D"
PI <- pi
Copy link
Member

Choose a reason for hiding this comment

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

Good -- PI <- pi is better than PI <- 3.141592 -- but still a placeholder until PConstants are defined and loaded outside the example sketches.

In the meantime perhaps all examples using PI, HALF_PI, TWO_PI etc. should be updated to be defined in terms of the R built-in pi.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is not what we need. In GSoC we will support the variables so it is only temporary. Both pi and 3.1415 are fine, IMO.

Copy link
Member Author

Choose a reason for hiding this comment

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

And in August we will remove all this tricks from example code.

Copy link
Member

Choose a reason for hiding this comment

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

As long as they are temporary and all get cleaned up later the inconsistency now is fine.

@@ -0,0 +1,2 @@
test:
reference: https://processing.org/reference/images/ambientLight_0.png
Copy link
Member

Choose a reason for hiding this comment

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

I notice that the original Processing reference example images are zero-indexed, so the reference directory names and .rpde files are zero-indexed. I believe that the first batch of examples were 1-indexed, like:

examples/reference/bezierTangent/bezierTangent1/bezierTangent1.rpde

examples/reference/bezierTangent/bezierTangent2/bezierTangent2.rpde

Should all these old files / paths be renamed to match the pattern?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it doesn't matter since it works well now :)

Copy link
Member

@jeremydouglass jeremydouglass Jun 13, 2017

Choose a reason for hiding this comment

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

I took another look at the original Processing reference and it is not consistent either -- some end in _, some end in _0, and some have numbers in the middle of the name... some are 0-indexed and some are (implicitly) 1-indexed.

For example, the reference images for arc() are arc_.png, arc_2.png, arc_3.png etc....

Given that the original material isn't consistent we don't need to worry right now about the new material being consistent -- it won't help us automate. So let's leave things as they are.

Copy link
Member Author

Choose a reason for hiding this comment

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

😄 Yeah, it can't be automated, so we need to write .test.yml manually.

gaocegege added 12 commits June 13, 2017 17:12
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
@gaocegege gaocegege merged commit 80fca43 into master Jun 13, 2017
@gaocegege gaocegege deleted the reference/light-camera branch June 13, 2017 10:08
@gaocegege gaocegege added this to the Evaluation 1 milestone Jun 14, 2017
@gaocegege gaocegege changed the title WIP: Add reference code for Lights, Camera Add reference code for Lights, Camera Jul 11, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants