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.

Sign up for GitHub

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

Fix WebGL vertex property issue #7605

Open
wants to merge 3 commits into
base: dev-2.0
Choose a base branch
from
Open

Fix WebGL vertex property issue #7605

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64188e1
Fix WebGL vertex property issue
ImRAJAS-SAMSE Mar 6, 2025
48ca792
Fix vertex properties handling
ImRAJAS-SAMSE Mar 8, 2025
b6491c2
Update p5.Geometry.js commets
ImRAJAS-SAMSE Mar 8, 2025
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
216 changes: 106 additions & 110 deletions src/webgl/p5.Geometry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1369,126 +1369,122 @@ class Geometry {
const potentialCaps = new Map();
const connected = new Set();
let lastValidDir;

for (let i = 0; i < this.edges.length; i++) {
const prevEdge = this.edges[i - 1];
const currEdge = this.edges[i];
const begin = this.vertices[currEdge[0]];
const end = this.vertices[currEdge[1]];
const prevColor = (this.vertexStrokeColors.length > 0 && prevEdge)
? this.vertexStrokeColors.slice(
prevEdge[1] * 4,
(prevEdge[1] + 1) * 4
)
: [0, 0, 0, 0];
const fromColor = this.vertexStrokeColors.length > 0
? this.vertexStrokeColors.slice(
currEdge[0] * 4,
(currEdge[0] + 1) * 4
)
: [0, 0, 0, 0];
const toColor = this.vertexStrokeColors.length > 0
? this.vertexStrokeColors.slice(
currEdge[1] * 4,
(currEdge[1] + 1) * 4
)
: [0, 0, 0, 0];
const dir = end
.copy()
.sub(begin)
.normalize();
const dirOK = dir.magSq() > 0;
if (dirOK) {
this._addSegment(begin, end, fromColor, toColor, dir);
}
if (!this.renderer?._simpleLines) {
if (i > 0 && prevEdge[1] === currEdge[0]) {
if (!connected.has(currEdge[0])) {
connected.add(currEdge[0]);
potentialCaps.delete(currEdge[0]);
// Add a join if this segment shares a vertex with the previous. Skip
// actually adding join vertices if either the previous segment or this
// one has a length of 0.
//
// Don't add a join if the tangents point in the same direction, which
// would mean the edges line up exactly, and there is no need for a join.
if (lastValidDir && dirOK && dir.dot(lastValidDir) < 1 - 1e-8) {
this._addJoin(begin, lastValidDir, dir, fromColor);
}
}
} else {
// Start a new line
if (dirOK && !connected.has(currEdge[0])) {
const existingCap = potentialCaps.get(currEdge[0]);
if (existingCap) {
this._addJoin(
begin,
existingCap.dir,
dir,
fromColor
);
potentialCaps.delete(currEdge[0]);
connected.add(currEdge[0]);
} else {
potentialCaps.set(currEdge[0], {
point: begin,
dir: dir.copy().mult(-1),
color: fromColor
});
}
}
if (lastValidDir && !connected.has(prevEdge[1])) {
const existingCap = potentialCaps.get(prevEdge[1]);
if (existingCap) {
this._addJoin(
this.vertices[prevEdge[1]],
lastValidDir,
existingCap.dir.copy().mult(-1),
prevColor
);
potentialCaps.delete(prevEdge[1]);
connected.add(prevEdge[1]);
} else {
// Close off the last segment with a cap
potentialCaps.set(prevEdge[1], {
point: this.vertices[prevEdge[1]],
dir: lastValidDir,
color: prevColor
});
}
lastValidDir = undefined;
}
const prevEdge = this.edges[i - 1];
const currEdge = this.edges[i];
const begin = this.vertices[currEdge[0]];
const end = this.vertices[currEdge[1]];

// Ensuring vertex properties are copied for stroke vertices
if (this.vertexProperties && this.vertexProperties[currEdge[0]]) {
this.vertexProperties.push([...this.vertexProperties[currEdge[0]]]);
}

if (i === this.edges.length - 1 && !connected.has(currEdge[1])) {
const existingCap = potentialCaps.get(currEdge[1]);
if (existingCap) {
this._addJoin(
end,
dir,
existingCap.dir.copy().mult(-1),
toColor
);
potentialCaps.delete(currEdge[1]);
connected.add(currEdge[1]);
} else {
potentialCaps.set(currEdge[1], {
point: end,
dir,
color: toColor
});
}
if (this.vertexProperties && this.vertexProperties[currEdge[1]]) {
this.vertexProperties.push([...this.vertexProperties[currEdge[1]]]);
}

const prevColor = (this.vertexStrokeColors.length > 0 && prevEdge)
? this.vertexStrokeColors.slice(
prevEdge[1] * 4,
(prevEdge[1] + 1) * 4
)
: [0, 0, 0, 0];

const fromColor = this.vertexStrokeColors.length > 0
? this.vertexStrokeColors.slice(
currEdge[0] * 4,
(currEdge[0] + 1) * 4
)
: [0, 0, 0, 0];

const toColor = this.vertexStrokeColors.length > 0
? this.vertexStrokeColors.slice(
currEdge[1] * 4,
(currEdge[1] + 1) * 4
)
: [0, 0, 0, 0];

const dir = end.copy().sub(begin).normalize();
const dirOK = dir.magSq() > 0;
if (dirOK) {
lastValidDir = dir;
this._addSegment(begin, end, fromColor, toColor, dir);
}

if (!this.renderer?._simpleLines) {
if (i > 0 && prevEdge[1] === currEdge[0]) {
if (!connected.has(currEdge[0])) {
connected.add(currEdge[0]);
potentialCaps.delete(currEdge[0]);
if (lastValidDir && dirOK && dir.dot(lastValidDir) < 1 - 1e-8) {
this._addJoin(begin, lastValidDir, dir, fromColor);
}
}
} else {
if (dirOK && !connected.has(currEdge[0])) {
const existingCap = potentialCaps.get(currEdge[0]);
if (existingCap) {
this._addJoin(begin, existingCap.dir, dir, fromColor);
potentialCaps.delete(currEdge[0]);
connected.add(currEdge[0]);
} else {
potentialCaps.set(currEdge[0], {
point: begin,
dir: dir.copy().mult(-1),
color: fromColor
});
}
}

if (lastValidDir && !connected.has(prevEdge[1])) {
const existingCap = potentialCaps.get(prevEdge[1]);
if (existingCap) {
this._addJoin(
this.vertices[prevEdge[1]],
lastValidDir,
existingCap.dir.copy().mult(-1),
prevColor
);
potentialCaps.delete(prevEdge[1]);
connected.add(prevEdge[1]);
} else {
potentialCaps.set(prevEdge[1], {
point: this.vertices[prevEdge[1]],
dir: lastValidDir,
color: prevColor
});
}
lastValidDir = undefined;
}
}

if (i === this.edges.length - 1 && !connected.has(currEdge[1])) {
const existingCap = potentialCaps.get(currEdge[1]);
if (existingCap) {
this._addJoin(end, dir, existingCap.dir.copy().mult(-1), toColor);
potentialCaps.delete(currEdge[1]);
connected.add(currEdge[1]);
} else {
potentialCaps.set(currEdge[1], {
point: end,
dir,
color: toColor
});
}
}

if (dirOK) {
lastValidDir = dir;
}
}
}
}

for (const { point, dir, color } of potentialCaps.values()) {
this._addCap(point, dir, color);
this._addCap(point, dir, color);
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to how functions like addCap, addSegment, etc copy the positions and colors into new buffers, I think we'd have to do the same for vertex properties. The reason for this is that while the data is initially recorded per point on the line, those points then get turned into triangles to give the line thickness, so we end up having to repeat the data a few times for each point in the triangle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll update the implementation

}
return this;
}
}


/**
* Adds the vertices and vertex attributes for two triangles making a rectangle
Expand Down