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

Adding strokeMode() to access SIMPLE lines. #7390

Merged
merged 3 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/core/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,18 @@ export const WEBGL2 = Symbol('webgl2');
* @final
*/
export const ARROW = 'default';

/**
* @property {String} SIMPLE
* @final
*/
export const SIMPLE = 'simple';
/**
* @property {String} FULLY
* @final
*/
export const FULLY = 'fully';
Copy link
Contributor

Choose a reason for hiding this comment

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

A minor note, could we call this FULL instead of FULLY to use the same adjective form as SIMPLE?


/**
* @typedef {'crosshair'} CROSS
* @property {CROSS} CROSS
Expand Down Expand Up @@ -1318,4 +1330,4 @@ export const HALF_FLOAT = 'half-float';
* @property {RGBA} RGBA
* @final
*/
export const RGBA = 'rgba';
export const RGBA = 'rgba';
141 changes: 120 additions & 21 deletions src/webgl/3d_primitives.js
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,94 @@ function primitives3D(p5, fn){
return this._renderer.endGeometry();
};


/**
* Sets the stroke rendering mode to balance performance and visual features when drawing lines.
*
* `strokeMode()` offers two modes:
*
* - `SIMPLE`: Optimizes for speed by disabling caps, joins, and stroke color features.
* Use this mode for faster line rendering when these visual details are unnecessary.
* - `FULLY`: Enables caps, joins, and stroke color for lines.
* This mode provides enhanced visuals but may reduce performance due to additional processing.
*
* Choose the mode that best suits your application's needs to either improve rendering speed or enhance visual quality.
*
* @method strokeMode
* @param {string} mode - The stroke mode to set. Possible values are:
* - `'SIMPLE'`: Fast rendering without caps, joins, or stroke color.
* - `'FULLY'`: Detailed rendering with caps, joins, and stroke color.
*
* @example
* <div>
* <code>
* function setup() {
* createCanvas(100, 100, WEBGL);
* strokeWeight(10);
* stroke(0);
*
* describe('A red, horizontal, rectangular shape with rounded edges (resembling a pill or capsule) in a grey background');
* }
*
* function draw() {
* strokeMode(FULLY); // Enables detailed rendering with caps, joins, and stroke color.
* stroke('red');
*
* background(128);
*
* let centerX = 0;
* let centerY = 0;
*
* // Length of the small centered line
* let lineLength = 50;
*
* beginShape(LINES);
* vertex(centerX - lineLength / 2, centerY, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you feel about having a 3D shape like a default sphere(), and maybe a single Bezier curve in each of these examples? I feel like those are two cases where the difference is clear: the sphere probably looks the same regardless, and the curve (especially with a higher stroke weight) will look very disjoint without full curve decorations.

We can also make the examples more than 100x100 if that makes it easier to see since I think the new site's layout accommodates that decently.

* vertex(centerX + lineLength / 2, centerY, 0);
* endShape();
* }
* </code>
* </div>
*
* <div>
* <code>
* function setup() {
* createCanvas(100, 100, WEBGL);
* strokeWeight(10);
* stroke(0);
*
* describe('A black, horizontal, rectangular shape without rounded edges in a grey background');
* }
*
* function draw() {
* strokeMode(SIMPLE); // Enables detailed rendering with caps, joins, and stroke color.
* stroke(`red`);
* background(128);
*
* let centerX = 0;
* let centerY = 0;
*
* // Length of the small centered line
* let lineLength = 50;
*
* beginShape(LINES);
* vertex(centerX - lineLength / 2, centerY, 0);
* vertex(centerX + lineLength / 2, centerY, 0);
* endShape();
* }
* </code>
* </div>
*/

fn.strokeMode = function(mode){
if(mode === constants.SIMPLE){
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make this return the current mode if we call it with no arguments?

this._renderer._simpleLines = true;
} else if (mode === constants.FULLY) {
this._renderer._simpleLines = false;
} else {
throw Error('no such parameter');
}
}
/**
* Creates a custom <a href="#/p5.Geometry">p5.Geometry</a> object from
* simpler 3D shapes.
Expand Down Expand Up @@ -2109,7 +2197,7 @@ function primitives3D(p5, fn){
this.faces = [[0, 1, 2]];
this.uvs = [0, 0, 1, 0, 1, 1];
};
const triGeom = new Geometry(1, 1, _triangle);
const triGeom = new Geometry(1, 1, _triangle, this);
triGeom._edgesToVertices();
triGeom.computeNormals();
triGeom.gid = gid;
Expand Down Expand Up @@ -2245,7 +2333,7 @@ function primitives3D(p5, fn){
}
};

const arcGeom = new Geometry(detail, 1, _arc);
const arcGeom = new Geometry(detail, 1, _arc, this);
arcGeom.computeNormals();

if (detail <= 50) {
Expand Down Expand Up @@ -2308,7 +2396,7 @@ function primitives3D(p5, fn){
];
}
};
const rectGeom = new Geometry(detailX, detailY, _rect);
const rectGeom = new Geometry(detailX, detailY, _rect, this);
rectGeom
.computeFaces()
.computeNormals()
Expand Down Expand Up @@ -2440,7 +2528,7 @@ function primitives3D(p5, fn){
this.uvs.push([pctx, pcty]);
}
}
});
}, this);

quadGeom.faces = [];
for(let y = 0; y < detailY-1; y++){
Expand Down Expand Up @@ -3362,7 +3450,7 @@ function primitives3D(p5, fn){
}
}
};
const planeGeom = new Geometry(detailX, detailY, _plane);
const planeGeom = new Geometry(detailX, detailY, _plane, this);
planeGeom.computeFaces().computeNormals();
if (detailX <= 1 && detailY <= 1) {
planeGeom._makeTriangleEdges()._edgesToVertices();
Expand Down Expand Up @@ -3442,7 +3530,7 @@ function primitives3D(p5, fn){
this.faces.push([v + 2, v + 1, v + 3]);
});
};
const boxGeom = new Geometry(detailX, detailY, _box);
const boxGeom = new Geometry(detailX, detailY, _box, this);
boxGeom.computeNormals();
if (detailX <= 4 && detailY <= 4) {
boxGeom._edgesToVertices();
Expand Down Expand Up @@ -3498,7 +3586,7 @@ function primitives3D(p5, fn){
}
}
};
const ellipsoidGeom = new Geometry(detailX, detailY, _ellipsoid);
const ellipsoidGeom = new Geometry(detailX, detailY, _ellipsoid, this);
ellipsoidGeom.computeFaces();
if (detailX <= 24 && detailY <= 24) {
ellipsoidGeom._makeTriangleEdges()._edgesToVertices();
Expand All @@ -3525,17 +3613,18 @@ function primitives3D(p5, fn){
) {
const gid = `cylinder|${detailX}|${detailY}|${bottomCap}|${topCap}`;
if (!this.geometryInHash(gid)) {
const cylinderGeom = new p5.Geometry(detailX, detailY);
_truncatedCone.call(
cylinderGeom,
1,
1,
1,
detailX,
detailY,
bottomCap,
topCap
);
const cylinderGeom = new p5.Geometry(detailX, detailY, function() {
_truncatedCone.call(
this,
1,
1,
1,
detailX,
detailY,
bottomCap,
topCap
);
}, this);
// normals are computed in call to _truncatedCone
if (detailX <= 24 && detailY <= 16) {
cylinderGeom._makeTriangleEdges()._edgesToVertices();
Expand All @@ -3561,8 +3650,18 @@ function primitives3D(p5, fn){
) {
const gid = `cone|${detailX}|${detailY}|${cap}`;
if (!this.geometryInHash(gid)) {
const coneGeom = new Geometry(detailX, detailY);
_truncatedCone.call(coneGeom, 1, 0, 1, detailX, detailY, cap, false);
const coneGeom = new Geometry(detailX, detailY, function() {
_truncatedCone.call(
this,
1,
0,
1,
detailX,
detailY,
cap,
false
);
}, this);
if (detailX <= 24 && detailY <= 16) {
coneGeom._makeTriangleEdges()._edgesToVertices();
} else if (this.states.doStroke) {
Expand Down Expand Up @@ -3624,7 +3723,7 @@ function primitives3D(p5, fn){
}
}
};
const torusGeom = new Geometry(detailX, detailY, _torus);
const torusGeom = new Geometry(detailX, detailY, _torus, this);
torusGeom.computeFaces();
if (detailX <= 24 && detailY <= 16) {
torusGeom._makeTriangleEdges()._edgesToVertices();
Expand Down
2 changes: 1 addition & 1 deletion src/webgl/GeometryBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class GeometryBuilder {
renderer._pInst.push();
this.identityMatrix = new Matrix();
renderer.states.uModelMatrix = new Matrix();
this.geometry = new Geometry();
this.geometry = new Geometry(undefined, undefined, undefined, this.renderer);
this.geometry.gid = `_p5_GeometryBuilder_${GeometryBuilder.nextGeometryId}`;
GeometryBuilder.nextGeometryId++;
this.hasTransform = false;
Expand Down
2 changes: 1 addition & 1 deletion src/webgl/ShapeBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class ShapeBuilder {
constructor(renderer) {
this.renderer = renderer;
this.shapeMode = constants.TESS;
this.geometry = new Geometry();
this.geometry = new Geometry(undefined, undefined, undefined, this.renderer);
this.geometry.gid = '__IMMEDIATE_MODE_GEOMETRY__';

this.contourIndices = [];
Expand Down
2 changes: 1 addition & 1 deletion src/webgl/loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ function loading(p5, fn){
fileType = '.obj';
}

const model = new Geometry();
const model = new Geometry(undefined, undefined, undefined, this._renderer);
model.gid = `${path}|${normalize}`;

async function getMaterials(lines) {
Expand Down
8 changes: 6 additions & 2 deletions src/webgl/p5.Geometry.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
import { Vector } from '../math/p5.Vector';

class Geometry {
constructor(detailX, detailY, callback) {
constructor(detailX, detailY, callback, renderer) {
this.renderer = renderer;
this.vertices = [];

this.boundingBoxCache = null;
Expand Down Expand Up @@ -1398,7 +1399,7 @@
if (dirOK) {
this._addSegment(begin, end, fromColor, toColor, dir);
}

if(!this.renderer._simpleLines){
Copy link
Contributor

Choose a reason for hiding this comment

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

Since p5.Geometry created manually won't have a renderer passed in, I think we need to handle the case where there is no renderer, and mayube default to the full line behaviour:

Suggested change
if(!this.renderer._simpleLines){
if(!this.renderer?._simpleLines){

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, do you think we could intent the block inside this to keep the indentation consistent everywhere?

if (i > 0 && prevEdge[1] === currEdge[0]) {
if (!connected.has(currEdge[0])) {
connected.add(currEdge[0]);
Expand Down Expand Up @@ -1481,6 +1482,7 @@
lastValidDir = dir;
}
}
}
for (const { point, dir, color } of potentialCaps.values()) {
this._addCap(point, dir, color);
}
Expand Down Expand Up @@ -1520,6 +1522,7 @@
}
}
this.lineVertices.push(...a, ...b, ...a, ...b, ...b, ...a);
if(!this.renderer._simpleLines){

Check failure on line 1525 in src/webgl/p5.Geometry.js

View workflow job for this annotation

GitHub Actions / test

test/unit/webgl/p5.RendererGL.js > p5.RendererGL > interpolation of vertex colors > geom with vertex colors use their color (noLight)

TypeError: Cannot read properties of undefined (reading '_simpleLines') ❯ Geometry._addSegment src/webgl/p5.Geometry.js:1525:23 ❯ Geometry._edgesToVertices src/webgl/p5.Geometry.js:1400:14 ❯ fn.model src/webgl/loading.js:1109:15 ❯ test/unit/webgl/p5.RendererGL.js:2191:12

Check failure on line 1525 in src/webgl/p5.Geometry.js

View workflow job for this annotation

GitHub Actions / test

test/unit/webgl/p5.RendererGL.js > p5.RendererGL > interpolation of vertex colors > geom with vertex colors use their color (light)

TypeError: Cannot read properties of undefined (reading '_simpleLines') ❯ Geometry._addSegment src/webgl/p5.Geometry.js:1525:23 ❯ Geometry._edgesToVertices src/webgl/p5.Geometry.js:1400:14 ❯ fn.model src/webgl/loading.js:1109:15 ❯ test/unit/webgl/p5.RendererGL.js:2221:12
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly here, we might need to check for the existence of the renderer first:

Suggested change
if(!this.renderer._simpleLines){
if(!this.renderer?._simpleLines){

this.lineVertexColors.push(
...fromColor,
...toColor,
Expand All @@ -1528,6 +1531,7 @@
...toColor,
...fromColor
);
}
return this;
}

Expand Down
4 changes: 4 additions & 0 deletions src/webgl/p5.RendererGL.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ class RendererGL extends Renderer {
// erasing
this._isErasing = false;

// simple lines
this._simpleLines = false;

// clipping
this._clipDepths = [];
this._isClipApplied = false;
Expand Down Expand Up @@ -2150,6 +2153,7 @@ class RendererGL extends Renderer {

_setStrokeUniforms(strokeShader) {
// set the uniform values
strokeShader.setUniform('uSimpleLines', this._simpleLines);
strokeShader.setUniform('uUseLineColor', this._useLineColor);
strokeShader.setUniform('uMaterialColor', this.states.curStrokeColor);
strokeShader.setUniform('uStrokeWeight', this.curStrokeWeight);
Expand Down
6 changes: 5 additions & 1 deletion src/webgl/shaders/line.vert
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ uniform mat4 uProjectionMatrix;
uniform float uStrokeWeight;

uniform bool uUseLineColor;
uniform bool uSimpleLines;
uniform vec4 uMaterialColor;

uniform vec4 uViewport;
Expand Down Expand Up @@ -67,6 +68,8 @@ vec2 lineIntersection(vec2 aPoint, vec2 aDir, vec2 bPoint, vec2 bDir) {

void main() {
HOOK_beforeVertex();

if(!uSimpleLines){
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we indent the block below?

// Caps have one of either the in or out tangent set to 0
vCap = (aTangentIn == vec3(0.)) != (aTangentOut == (vec3(0.)))
? 1. : 0.;
Expand All @@ -77,6 +80,7 @@ void main() {
aTangentOut != vec3(0.) &&
aTangentIn != aTangentOut
) ? 1. : 0.;
}

vec4 localPosition = vec4(HOOK_getLocalPosition(aPosition.xyz), 1.);
vec4 posp = vec4(HOOK_getWorldPosition((uModelViewMatrix * localPosition).xyz), 1.);
Expand Down Expand Up @@ -171,7 +175,7 @@ void main() {
}

vec2 offset;
if (vJoin == 1.) {
if (vJoin == 1. && !uSimpleLines) {
vTangent = normalize(tangentIn + tangentOut);
vec2 normalIn = vec2(-tangentIn.y, tangentIn.x);
vec2 normalOut = vec2(-tangentOut.y, tangentOut.x);
Expand Down
2 changes: 1 addition & 1 deletion src/webgl/text.js
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ function text(p5, fn){
this.uvs.push(j, i);
}
}
}));
}, this) );
geom.computeFaces().computeNormals();
g = this.geometryBufferCache.ensureCached(geom);
}
Expand Down
2 changes: 1 addition & 1 deletion test/unit/webgl/p5.Geometry.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ suite('p5.Geometry', function() {
let geom;

beforeEach(function() {
geom = new p5.Geometry();
geom = new p5.Geometry(undefined, undefined, undefined, myp5._renderer);
vi.spyOn(geom, '_addCap');
vi.spyOn(geom, '_addJoin');
vi.spyOn(geom, '_addSegment');
Expand Down
Loading
Loading