Defaults for buildGeometry

[wagedu]

Increasing Access

More intuitive use of the command.

Most appropriate sub-area of p5.js?

• [ ] Accessibility
• [ ] Color
• [ ] Core/Environment/Rendering
• [ ] Data
• [ ] DOM
• [ ] Events
• [ ] Image
• [ ] IO
• [ ] Math
• [ ] Typography
• [ ] Utilities
• [X] WebGL
• [ ] Build Process
• [ ] Unit Testing
• [ ] Internalization
• [ ] Friendly Errors
• [ ] Other (specify if possible)

Feature enhancement details

Currently buildGeometry creates models affected by the context's open settings (fill, rectMode etc). ie if I have fill('blue') declared before I start a buildGeometry, the model will have the setting "fill = blue". Also, by default, buildGeometry forces a call to model.disableColor() if one wants to use fill on the model.

I feel like the default for buildGeometry should be vanilla. A vanilla geom that I can affect with fill is (probably) more frequent than a custom colored one where I want to disableColor before using fill on it (or rectMode, etc) - Specially because buildGeometry doesn't accept all params (no stroke, for instance), so it gets difficult to track what is being affected and what is not. Then, only in cases where I want a non-vanilla model, it's up to me/user to pass "extra parameters" as modifiers when starting buildGeometry()

[davepagurek]

Going to tag the other WebGL stewards too in case any of you have opinions on what the defaults should be: @aferriss, @aceslowman, @ShenpaiSharma, @ChihYungChang, @teragramgius, @JazerUCSB, @richardegil, @itsjoopark, @Gaurav-1306, @jeanetteandrews

Assuming we do want to change the defaults: one way to do this could be to add a settings object to buildGeometry and beginGeometry to change how it interprets the drawing calls within it. If we default to excluding colors, it could look something like this:

const geom = buildGeometry(() => {
  // ...
}, { includeColors: true });

// or:

beginGeometry({ includeColors: true });
// ...
const geom = endGeometry();

How do you feel about an API like that?

[aferriss]

A settings object sounds fine to me. What other settings would we add? If it's just a single one, it probably doesn't need to be an object, although I suppose that does future proof it somewhat if more options are added in the future.

[davepagurek]

One thing that comes to mind is possibly something to do with how edges are handled. Currently, edges display incorrectly if you create geometry without a stroke enabled, but then try to draw it later with strokes. One option is to always generate strokes when building geometry, but if this adds a significant time cost for people who won't ever use it, it might make sense to add a setting to disable it.

[perminder-17]

Hello @wagedu , Thank you for raising this point. I'd also like to express my gratitude to @davepagurek and @aferriss for sharing their insights on the topic. While working on the clearColors method in this GitHub issue https://github.com/processing/p5.js/pull/6498, I genuinely considered transitioning it to a vanilla format. If I am not wrong, Currently we have to do something like shape.clearColors() for clearing the internal colors.

const shape1 = buildGeometry(() => {
  fill('red');
  box();
});
shape1.clearColors();
fill('blue');
model(shape1); // the box should now be blue bcz we cleard its internal color.

But after the imlementation we will completely eliminate the concept of clearColors() making the code look like

const geom = buildGeometry(() => {
  // ...
}, { includeColors: true/false }); // by default it could be true.

// or:

beginGeometry({ includeColors: true/false });
// ...
const geom = endGeometry();

The vanilla approach seems satisfactory, especially considering our use of framebuffers with createFramebuffer(), where we employ the default vanilla setup like createFramebuffer({antialiasing: false/true}) for improved readability and flexibility in my opinion. Aligning buildGeometry with a similar vanilla approach would provide consistency. Users already familiar with the vanilla method of using createFramebuffer may find it more convenient to adopt a similar pattern across functions, making it easier for them to apply without the need to learn new conventions.

All I would say, I agree with @davepagurek suggestion about potentially adding new settings like enabling/disabling strokes. If we plan to introduce additional settings beyond just colors, it might be beneficial to maintain a vanilla format. However, if we are only dealing with color settings, the current(model.clearColors()) format could suffice.

[davepagurek]

For what it's worth, I think we'll still keep clearColors() around even if we switch to a settings object, as it will also be useful for clearing the built-in colors from loadModel if they aren't desired, once https://github.com/processing/p5.js/pull/6710 is merged in.

Also, in case it's relevant: currently, other material settings such as shininess are currently not included in buildGeometry because they don't quite fit within the data model we have right now. I also mentioned in this comment https://github.com/processing/p5.js/issues/6670#issuecomment-1871211149, but it could make sense at some point to have a data structure that contains (possibly multiple) p5.Geometry + material settings. That feels different enough that it would probably not be made from buildGeometry too, but a settings object feels like a pattern that would be useful in a builder function for that too.

[wagedu]

Hi @perminder-17 I've also enjoyed the way you've put it. A Vanilla setup and consistency sound super sexy. In a very geeky way, I know

For instance,buildGeometry({clearColors:true}) sounds clean to me.

It would be a way to be ready for what's coming. buildGeometry ,to me, is in the same "league" as frameBuffer and looks like it'll "grow". For instance buildGeometry({shininess: 20}) (or what you may be planning @davepagurek ) still looks "consistent", like something I would expect to manipulate that way. I mean, as long as it is one of the configurable settings, oc.

The question, though, is what defaults to use: If it DOES have embedded settings (on creation time or 'cos it's a model) they have to be manually disabled (clearColors, evtl clearShininess?) to allow for "permeability" from the scene/canvas settings. Otherwise, they remain set. I mean, they were set, I guess with some expectation, otherwise, don't set them, right?

But the "undefined" (not-set?) settings start blank + whenever the model is called, they are permeable to the canvas/scene settings.

**Just in case, by permeable from the canvas settings I mean affected by the canvas setting. Like being affected (or not) by a fill('red') for instance

[perminder-17]

Hi @wagedu , thanks for sharing your thoughts on this topic. I must say, our ideas align quite well, haha. I understand your point, and it resonates with me. Regarding the question of default settings, I completely agree with your suggestion to disable all default settings. Perhaps, if a user calls the buildGeometry function without any parameters, like buildGeometry(){...}, it should default to using the canvas settings. This approach makes sense, especially since we currently don't have many settings to include, maybe something like shininess in the future. Keeping the default settings disable is good. Also, considering that the createFramebuffer function aligns with a similar approach by taking the width, height, and density of the canvas by default, staying consistent in this league sounds good to me.

One question arises for me here, and I know it might sound silly, but it seems like something worth considering for an improved user experience. When creating multiple models on a canvas, calling buildGeometry multiple times, do you think we might encounter difficulties? In my opinion, I would prefer to include colors inside buildGeometry for multiple model and for that I have to do something like buildGeometry({includeColors: true}) multiple times. Otherwise, it could be challenging for me to keep track of colors for each model if we rely on canvas settings colors. How do you propose we handle such cases? Not a big deal, Maybe it basically depends on user to user .

[wagedu]

Hello @perminder-17 glad I didn't write a bunch of madness :)

If your question is silly, then I'm Mr Silly. Defaults are -to me- the hardest choice In this case, though, I've been using @davepagurek 's previous versions of buildGeometry (and frameBuffer and anything this gift of a man man creates) A LOT as external libraries, and once even created hundreds/thousands of different models in a single project. Meaning I've become an entitled brat full of opinions, lol

Now, apologies if I use "automatically" as if it's something super easy to do. I don't know how the models are built internally. So, please take it as a suggestion from someone ignorant as to how easy/hard it is to implement.

I'd go with your suggestion UNLESS it is possible to detect if any colors have been set or not. Then I'd use nothing. ie. if the user NEVER uses fill INSIDE buildGeometry, it's safe to assume they DON'T want custom fills for the model. So the model is automatically created with includeColors: false. But if the user DOES use fill INSIDE buildGeometry. Then I'd assume they DO want custom fills for the model. So the model is automatically created with includeColors: true *Automatically = buildGeometry checks if fill is ever called, then sets includeColors to true/false accordingly?

Which also arises another question: Would it mean that the user can, at any time, use myModel.includeColors = false to disable internal colors and allow the specific model be affected by the canvas settings' colors? And then myModel.includeColors = true to re-enable (in case there are includedColors) ? From my very personal PoV, that sounds like a great feature to have. And sounds in-logic. *OOOOPS, EDIT: I just realized that would also address @davepagurek suggestion of "keep clearColors() around even if we switch to a settings object, as it will also be useful for clearing the built-in colors from loadModel if they aren't desired". As I described above, I'd find more useful an on/off (of settings/info already saved to the model) instead of a clear

Bonus question: how does internalColors or modelColors sound?

Cheers, have a great weekend!

[wagedu]

OOOOPS 2: sorry @perminder-17 and @davepagurek I did a crucial EDIT to my previous answer, but I think you'll receive the NON-edited version by email... Which means you'll have to come here to see it entirely :( Apologies Just in case: it's in regards to @davepagurek suggestion of "keep clearColors() around even if..." Cheers, sorry again

[perminder-17]

Thank you once again, @wagedu , for sharing your insightful thoughts and taking the time to discuss this topic. As you mentioned earlier, the buildGeometry function is expected to grow, and I share the same sentiment. We will going to be the one who will help making it grow hahaha

Then I'd use nothing. ie. if the user NEVER uses fill INSIDE buildGeometry, it's safe to assume they DON'T want custom fills for the model. So the model is automatically created with includeColors: false. But if the user DOES use fill INSIDE buildGeometry. Then I'd assume they DO want custom fills for the model. So the model is automatically created with includeColors: true

Regarding the use of the word "automatically" as mentioned above, where buildGeometry checks if fill is ever called and sets includeColors to true/false accordingly, this sounds fantastic. I would love to discuss it further, but before diving into the discusion on this, I'm unsure about its feasibility. I'm unsure how we'll distinguish between the fill() used on the canvas and within buildGeometry. Previously, before the introduction of the clearColors() function, users have to make all vertices with same color to clear internal colors to use canvas fill colors. Now, with clearColors() available, I'm open to the idea if it's technically possible. I'll tag @davepagurek for his input on the feasibility.

[davepagurek]

I think it's feasible: in beginGeometry, we could set this.hasChangedColor = false, and then in every call to fill, we could set this.hasChangedColor = true. Then GeometryBuilder could check the value of hasChangedColor to see whether to include or ignore the color.

My initial reasoning for trying to not be fancy is because some other properties get set outside of buildGeometry and affect its result. Setting a texture outside of buildGeometry affects the texture coordinates stored within it (by default, textures are specified in pixels relative to the current texture, and then get normalized by its size) but the texture itself currently does not get stored with the geometry. This may change in the future if we make a data structure that stores both geometry + material settings though. Potentially that can employ something similar where we only include it if it's set within buildGeometry or whatever this future geometry+material builder is called? So that would mean that this pattern could be extended as we expand our capabilities.

[wagedu]

Again, apologies in advance for questions based on lack of knowledge. Also, I'll use again the term "permeable' to mean "uses whatever fill color that is set in the canvas". Shorter. And I'll complicate things by considering custom geometries with more than one color/setting.

My suggested "logic" comes from expecting the customGeoms to behave like native Geoms (as far as possible, oc)

function setup() {
  createCanvas(400, 400,WEBGL);
  noStroke();
  fill('orange')
  myG = buildGeometry(()=>{
    sphere(50); //fill NOT specified
    translate(0,50,0)
    fill('red')
    sphere(50)
  })
}

function draw() {
  background(220);
  // myG.clearColors();
  fill('blue')
  model(myG)
}

Assume I create a model called myG with 2 spheres in it. Only the second one has fill('red') set DURING creation. Currently, if I don't set the first sphere's color, it picks whatever color was in the canvas at the time of creation and hard-codes it into the model... In this case, orange. But... I don't like the idea of having to reset everything on the canvas prior to creating a model. I think the default should NOT be "canvas settings at the time of creation", but it should be "permeable". Like currently happens with a sphere or a cone or any native geom.

Currently, it will display one orange sphere (color picked from the canvas settings at model birth) and one red sphere no matter what, 'cos currently custom models are impermeable by default AND their unset settings are forcibly set (orange 1st sphere) Currently I must use myG.clearColors() to make the whole model permeable.

That makes me wonder 2 things:

• Shouldn't the first sphere (any part of a new geom) be permeable by default, unless I specify a setting for it? Currently it is hard-coded "canvas fill at the time of creation" (orange)
• Can't I get back to "Original settings" (permeable - red) after using myG.clearColors()? Wouldn't it be useful if, instead of "clear", we used some sort of "disable" that allows for a "re-enable"? ie. modelColors = true/false. No idea if it's possible/feasible or the model would need to be re-built. Just... would be nice.

= What I (super personal PoV again) would expect, considering native geoms' behavior is:

• NEVER get the 1st sphere filled orange. The canvas settings at creation time should NOT make it to the geometry.

• Any "part" of the geometry that is created without a specified fill, should be "permeable" not "hard-coded canvas fill at time of creation". ie, if I call my myG like this: fill('blue'); model(myG); I should get a blue sphere and a red sphere. (only the second sphere was created AFTER fill, only she has hard-coded color)

If if I call my model like this: myG.clearColors(); fill('blue'); model(myG); I get, as I would expect, a blue MODEL (bue sphere + blue sphere) Here I'd only add the suggestion of using myG.modelColors = false; instead of myG.clearColors(). If it's doable And then, if AFTER that I wanted the "modelColors" again, I'd call my model like this: myG.modelColors = true; fill('yellow'); model(myG); And get a yellow sphere and a red sphere. Again, I don't know if a "switch" is feasible