WIP: add WebGL debugMode() #3103
Conversation
kjhollen
left a comment
kjhollen
left a comment
There was a problem hiding this comment.
See API suggestion and validateParams discussion inline.
| * } | ||
| * } | ||
| * </code> | ||
| * </div> |
There was a problem hiding this comment.
each one of these needs an alt text description.
| * @param {Number} [zOff] | ||
| */ | ||
|
|
||
| p5.prototype.debugMode = function() { |
There was a problem hiding this comment.
I find myself wondering if this should be debug3dMode, but I think this ok to leave as-is
| normalMaterial(); | ||
| let cam = createCamera(); | ||
| cam.move(0, -this.height / 4, 0); | ||
| debugMode(); |
There was a problem hiding this comment.
just want to say that being able to comment this in or out in setup is a nice touch, and to have the flexibility of toggling with keypress is also great.
| * box(15, 30); | ||
| * // Press the spacebar to turn debugMode off! | ||
| * if (keyIsDown(32)) { | ||
| * debugMode(OFF); |
There was a problem hiding this comment.
in keeping with smooth/noSmooth — maybe debugMode / noDebugMode? then we can eliminate the OFF parameter.
There was a problem hiding this comment.
Yes, that is much better!
| * icon will be sized according to the current canvas size. Note that because the | ||
| * grid runs parallel to the default camera view, it is often helpful to use | ||
| * debugMode along with orbitControl to allow full view of the grid. | ||
| * @method debugMode |
There was a problem hiding this comment.
I had a look at the errors for the argument-less version. I think the issue is that _validateParameters doesn't expect the argument array to be null. I recall seeing a discussion about not generating an error when the user supplies an argument to an argumentless function, because the function will work and the error will confuse a new programmer (I think not generating one is the right call). but, this case is different because that's only one mode of operation. hmm. maybe there's a way to work around by supplying a default argument?
There was a problem hiding this comment.
This seems to have been fixed by adding 'either' after the @param {Constant} mode tag, per @Spongman's suggestion below.
| * box(15, 30); | ||
| * } | ||
| * </code> | ||
| * </div> |
There was a problem hiding this comment.
these all look great, but when I click on the canvas, everything disappears! any ideas? I don't see anything in the error log. I'm running with npm run grunt yui:dev
There was a problem hiding this comment.
woops, okay, it looks like I can't fully test this until we get 3088 in—I think it's ready to go, but just had one more quick question there. I'll take a closer look at this and the _orbit tests tomorrow morning!
There was a problem hiding this comment.
@AidanNelson I just merged #3088, do you want to go ahead and merge this branch with webgl-gsoc-2018 and push again?
There was a problem hiding this comment.
cool, confirming that this is resolved with a merge of the latest webgl-gsoc-2018 branch!
| // Lines along X axis | ||
| for (var q = 0; q <= numDivs; q++) { | ||
| this.strokeWeight(1); | ||
| this.stroke(0, 0, 0); |
There was a problem hiding this comment.
hmm, it could be good to set this color... black is a very common background color for 3D work. or could it just use the stroke color set by the user?
There was a problem hiding this comment.
Good point, that would be nice!
I've set it to use the last-set stroke color and stroke weight. This introduces an odd behavior when called without first setting a stroke, where it will use the stroke color last set by _axesIcon(), but I suspect this is fixed by #3076.
| let cam = createCamera(); | ||
| cam.move(0, -this.height / 4, 0); | ||
| debugMode(); | ||
| } |
There was a problem hiding this comment.
but also: I can't run this because it generates an error with validateParams in my default build...
There was a problem hiding this comment.
there's a bug in the validateParameters handling that causes a crash when the description of a {Constant}-typed parameter does not match the required format.
the fix in this case is to add the 'either' keyword before the list of valid values for the parameter ("either GRID, AXES, or OFF").
There was a problem hiding this comment.
@Spongman Yes, that seems to have fixed it! Is this bug documented anywhere? It would be helpful to have this somewhere in the developer docs.
There was a problem hiding this comment.
indeed, going to add that now...
There was a problem hiding this comment.
sorry, i should have done this a long time ago... #3122
|
|
||
| /** | ||
| * @method debugMode | ||
| * @param {Constant} mode GRID, AXES, or OFF |
There was a problem hiding this comment.
this should be
* @param {Constant} mode either GRID, AXES, or OFF
|
@kjhollen these most recent changes also simplify the argument-checking. Let me know if you prefer the previous version. And sorry about the number of commits! Let me know if you'd like me to rebase them... |
| }; | ||
|
|
||
| /** | ||
| * DebugMode() helps visualize 3D space by adding a grid to indicate where the |
There was a problem hiding this comment.
use lowercase 'd' here, sorry I didn't catch this one before!
| * </code> | ||
| * </div> | ||
| * @alt | ||
| * a 3D box is centered on a grid in a 3D sketch. there is also an icon |
There was a problem hiding this comment.
can you summarize what the icon is concisely? three lines + color + direction?
3 participants
This PR adds a
debugMode()function to the webGL mode. The function works to make visualizing 3D space somewhat easier by adding a grid 'ground' plane and an axes icon indicating the +X, +Y, and +Z directions. Also..._grid()and_axes()private functions to be added to preregisteredMethods bydebugMode().If there is a cleaner way to overload this function, please let me know.