Basic_viewer: fix wrong edge width and vertex size in line-width shader

[RajdeepKushwaha5]

Fix three bugs in the line-width rendering path that caused edges to appear at incorrect widths and made the size_edges() / size_vertices() API behave incorrectly or have no effect in certain modes.

---

Bug 1 — Quadratic dependence on u_PointSize in GEOMETRY_SOURCE_LINE_WIDTH

The perpendicular offset vector was computed as:

vec2 n0 = vec2(-v0.y, v0.x) * u_PointSize * 0.5;

and then scaled by gs_in[i].pointSize = u_PointSize / effectiveDistance, making the actual half-width proportional to u_PointSize². For scenes with large bounding-box diagonals this produced edges far wider than intended, and size_edges() had a super-linear effect.

Fix: remove u_PointSize from n0 so the half-width is linear: u_PointSize / (2 * effectiveDistance).

---

Bug 2 — Orthographic mode always produced width 1.0

In VERTEX_SOURCE_LINE_WIDTH, the orthographic branch set distance = u_PointSize, so pointSize = u_PointSize / u_PointSize = 1.0. The user-specified size was silently ignored in every 2D viewer.

Fix: use distance = 1.0 so pointSize = u_PointSize passes through directly as a screen-pixel width.

---

Bug 3 — Baked-in viewport dimensions

The u_Viewport uniform was always uploaded as the initial window size {500, 450}:

QVector2D viewport = { CGAL_BASIC_VIEWER_INIT_SIZE_X, CGAL_BASIC_VIEWER_INIT_SIZE_Y };

After any window resize the NDC↔pixel conversions inside the geometry shader used stale dimensions, producing incorrect edge geometry.

Fix: use this->width() / this->height() on every frame.

---

Files changed

• Basic_viewer/include/CGAL/Basic_shaders.h — VERTEX_SOURCE_LINE_WIDTH, GEOMETRY_SOURCE_LINE_WIDTH
• Basic_viewer/include/CGAL/Qt/Basic_viewer.h — viewport uniform in the edge-drawing renderer

Fixes #9327

[Copilot]

Pull request overview

Fixes the line-width rendering path in CGAL’s Basic Viewer so size_edges() / size_vertices() map correctly to on-screen pixel sizes across perspective vs. orthographic projections and after window resizes.

Changes:

• Correct orthographic line-width scaling so u_PointSize is no longer silently canceled out.
• Fix geometry-shader offset math to remove unintended quadratic dependence on u_PointSize.
• Upload dynamic viewport dimensions each frame for correct NDC↔pixel conversions after resize.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
Basic_viewer/include/CGAL/Qt/Basic_viewer.h	Updates u_Viewport uniform to use current widget dimensions instead of initial constants.
Basic_viewer/include/CGAL/Basic_shaders.h	Fixes line-width vertex/geometry shader math for correct pixel-width behavior in ortho and perspective.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

u_Viewport appears to be used as a screen-pixel scale in the line-width geometry shader. Using this->width()/this->height() here will be in Qt device-independent pixels, so on HiDPI displays the viewport uniform will be off by devicePixelRatio() and line widths will render incorrectly. Consider uploading framebuffer pixel dimensions instead (e.g., width()*devicePixelRatio() and height()*devicePixelRatio(), or use the camera’s pixel ratio if that’s the established source of truth).

Suggested change

	// Use the actual current viewport dimensions so that the line-width
	// geometry shader converts between NDC and screen-pixels correctly
	// even after the window has been resized.
	QVector2D viewport(static_cast<float>(this->width()),
	static_cast<float>(this->height()));
	// Use the actual current framebuffer pixel dimensions so that the line-width
	// geometry shader converts between NDC and screen-pixels correctly
	// even after the window has been resized and on HiDPI displays.
	const float pixel_ratio = static_cast<float>(this->devicePixelRatio());
	QVector2D viewport(static_cast<float>(this->width()) * pixel_ratio,
	static_cast<float>(this->height()) * pixel_ratio);

[gdamiand]

Thanks @RajdeepKushwaha5 for the patch.
Did you test the updated version of the basic viewer for different meshes, 2D and 3D, of different sizes?
Could you show some results please?

[aminkhalsi]

hi @gdamiand, I know this is addressed to @RajdeepKushwaha5, but having impelemented a similar patch I can share some of the results:
This is the default draw_lcc.cpp example with the patch:

This is the result of scaling it's size by 100 before the patch

This is the result of scaling it's size by 100 after the patch

for 2d scenes, the u_PointSize is set to 2.5 (isn't affected by the bbox size) and then divided by itself in the vertex shader resulting in gs_in[0,1].pointSize=1 and then they get scaled by the u_PointSize *0.5 ( because of n0). removing the multiplication by u_PointSize results in constant edge sizes. Also I think the multiplication by 0.5 should be removed and the n0 should be normalized since it makes more sense that the edge size is only affected by one variable gs_in[i].pointSize . Setting the distance in the vertex shader as 1.0 makes the edge size change with user input, but doesn't scale down when zooming out or scale up when zooming in.
As for changing the viewport I don't know what that does, I'll leave that to @RajdeepKushwaha5.
Note that the step used for increasing the edge and vertex sizes should be proportional to the bbox_size, since for large scenes stepping by 0.5 or a small constant has almost no effect.

[RajdeepKushwaha5]

Thanks @gdamiand! Yes, I tested the patch across different geometries and scales, both 2D and 3D.

Bug 1 — Quadratic edge width at large scale

Using draw_lcc with the hexahedron geometry scaled 100× (bbox diagonal ≈ 820).

Before — edges are massively blown out due to quadratic u_PointSize² dependence:

After — correct linear edge width, cube fully visible:

At normal scale (bbox diagonal ≈ 8.2), before and after both look correct — the bug only manifests at larger scales:

Before (normal scale):

After (normal scale):

---

Bug 2 — Orthographic mode ignores size_edges() / + key

Using draw_polygon_2 (2D polygon, orthographic projection).

Before — pressing + has no visible effect on edge width (pointSize was always 1.0):

After (default):

After (pressing + several times) — edges are now visibly thicker:

---

Bug 3 — Viewport resize

Also verified that resizing the window no longer breaks edge geometry. The previous code always used the initial {500, 450} constants; the patch now uses this->width() * devicePixelRatio() and this->height() * devicePixelRatio() each frame.

---

3D mesh — elephant.off

Test configurations:

• 3D perspective: LCC hexahedron at 1× and 100× scale, elephant.off
• 2D orthographic: Polygon_2 at 1× scale
• Edge size interaction: +/- keys tested in both 2D and 3D
• Window resize: Verified NDC↔pixel conversion stays correct after resize

[RajdeepKushwaha5]

hi @gdamiand, I know this is addressed to @RajdeepKushwaha5, but having impelemented a similar patch I can share some of the results: This is the default draw_lcc.cpp example with the patch: This is the result of scaling it's size by 100 before the patch This is the result of scaling it's size by 100 after the patch for 2d scenes, the u_PointSize is set to 2.5 (isn't affected by the bbox size) and then divided by itself in the vertex shader resulting in gs_in[0,1].pointSize=1 and then they get scaled by the u_PointSize *0.5 ( because of n0). removing the multiplication by u_PointSize results in constant edge sizes. Also I think the multiplication by 0.5 should be removed and the n0 should be normalized since it makes more sense that the edge size is only affected by one variable gs_in[i].pointSize . Setting the distance in the vertex shader as 1.0 makes the edge size change with user input, but doesn't scale down when zooming out or scale up when zooming in. As for changing the viewport I don't know what that does, I'll leave that to @RajdeepKushwaha5. Note that the step used for increasing the edge and vertex sizes should be proportional to the bbox_size, since for large scenes stepping by 0.5 or a small constant has almost no effect.

Thanks @aminkhalsi for testing and sharing results — glad the large-scale fix is confirmed on your side too!

A few clarifications on the technical points you raised:

On removing * 0.5 and normalizing n0:
v0 is already normalized (vec2 v0 = normalize(p1 - p0)), so n0 = vec2(-v0.y, v0.x) is already a unit vector. The * 0.5 converts the diameter (gs_in[i].pointSize) to a half-width — vertices are emitted at ±n0 * pointSize, so the total strip width = 2 × 0.5 × pointSize = pointSize. Removing * 0.5 would make edges twice as wide as the user-facing size_edges() value, which would be inconsistent with the point/vertex size semantics.

On distance = 1.0 in ortho mode:
In orthographic projection gl_Position.w is always 1.0, so the old code computed gl_PointSize = u_PointSize / u_PointSize which collapsed to 1.0 — completely ignoring the user's +/- input. Setting distance = 1.0 gives gl_PointSize = u_PointSize / 1.0 = u_PointSize, making edge width directly respond to user input. The tradeoff that edge width doesn't scale with camera zoom in ortho mode is deliberate: orthographic projections have no depth attenuation, so constant screen-pixel width is the expected behavior (matching how most CAD viewers handle it).

On the viewport fix:
The old code hardcoded the viewport as {500, 450} (the initial window size). After the user resizes the window, the geometry shader's ToScreenSpace() / ToWorldSpace() conversions used stale dimensions, causing edges to appear shifted or incorrectly scaled on resized windows. The fix reads the actual current widget size (with HiDPI scaling via devicePixelRatio()).

On step size proportional to bbox:
That's a good UX suggestion — the fixed 0.5 step for +/- can feel unresponsive for very large scenes. That could be a follow-up enhancement separate from this bug-fix PR.

[aminkhalsi]

ping @gdamiand

[sloriot]

Successfully tested in CGAL-6.3-Ic-1

[gdamiand]

Thanks @sloriot !