add line_width to ThinLines interfaces by koide3 · Pull Request #201 · koide3/iridescence

koide3 · 2026-05-01T06:21:33Z

No description provided.

Copilot

Pull request overview

This PR extends the ThinLines drawing APIs to support configuring line width (in pixels) across the C++ core and Python bindings, so callers can control ThinLines thickness without separate setter calls.

Changes:

Add line_width support to glk::ThinLines constructors and Python bindings.
Add line_width-aware overloads for update_thin_lines in LightViewerContext and AsyncLightViewerContext (C++ + Python).
Minor formatting cleanup in Python bindings (check_valid_voxels, whitespace).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`src/python/guik.cpp`	Adds Python overloads for `update_thin_lines(..., line_width, ...)` for both sync/async viewer contexts.
`src/python/glk.cpp`	Extends Python `glk.ThinLines` constructors to accept `line_width`; includes some formatting fixes.
`src/guik/viewer/light_viewer_context_util.cpp`	Implements `LightViewerContext::update_thin_lines` overload that applies `line_width`.
`src/guik/viewer/async_light_viewer_context.cpp`	Implements async `update_thin_lines` overload with `line_width` (and forwards legacy overload).
`src/glk/thin_lines.cpp`	Updates `glk::ThinLines` constructors to take and store `line_width`.
`include/guik/viewer/light_viewer_context.hpp`	Declares new `update_thin_lines` overloads/templates including `line_width`.
`include/guik/viewer/async_light_viewer_context.hpp`	Declares new async overloads/templates including `line_width`.
`include/glk/thin_lines.hpp`	Updates `glk::ThinLines` public constructor signatures to include `line_width` (defaulting to `1.0f`).

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

koide3 · 2026-05-01T09:25:52Z

@@ -177,17 +182,18 @@ void define_glk(py::module_& m) {
            colors[i] = glk::colormapf(glk::COLORMAP::TURBO, c[i]);
          }

-          return std::make_shared<glk::ThinLines>(vertices.data(), colors[0].data(), size, line_strip);
+          return std::make_shared<glk::ThinLines>(vertices.data(), colors[0].data(), size, line_strip, line_width);


@copilot apply changes based on this feedback

Fixed: c.size() is now included in the initial max and subsequent min computation for size, and the colors[0] access is guarded with size > 0. The redundant && i < c.size() in the loop condition was also removed since size is now bounded by c.size(). Commit: 8929991

koide3 · 2026-05-01T07:59:12Z

  std::vector<float> colors_buffer;
  if (colors) {
    colors_buffer.assign(colors, colors + 4 * num_vertices);
  }
  std::vector<unsigned int> indices_buffer(indices, indices + num_indices);



@copilot apply changes based on this feedback

Fixed: indices_buffer is now constructed only when indices != nullptr && num_indices > 0, eliminating the UB from null pointer arithmetic. Commit: 64f5904

koide3 · 2026-05-01T07:58:38Z

  guik::viewer()->invoke([=] {
    auto thin_lines = std::make_shared<glk::ThinLines>(vertices_buffer.data(), colors_buffer.data(), num_vertices, indices_buffer.data(), num_indices, line_strip);
+    thin_lines->set_line_width(line_width);


@copilot apply changes based on this feedback

Fixed: nullptr is now passed to glk::ThinLines when colors_buffer or indices_buffer are empty, rather than relying on .data() returning nullptr for empty vectors. Commit: 64f5904

koide3 · 2026-05-01T07:58:19Z

+    int num_indices,
+    bool line_strip,
+    float line_width,
+    const ShaderSetting& shader_setting);


@copilot apply changes based on this feedback

Added @param line_width Width of the lines in pixels. to the doc comment, consistent with async_light_viewer_context.hpp. Commit: 3d84c44

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…context.hpp Agent-Logs-Url: https://github.com/koide3/iridescence/sessions/44514c31-be2a-4517-b17d-4363bfaa0973 Co-authored-by: koide3 <31344317+koide3@users.noreply.github.com>

…ewer_context.cpp Agent-Logs-Url: https://github.com/koide3/iridescence/sessions/9dde4cd6-e065-42bf-a21c-0bfde09f1d09 Co-authored-by: koide3 <31344317+koide3@users.noreply.github.com>

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…but c non-empty Agent-Logs-Url: https://github.com/koide3/iridescence/sessions/5408ca4b-ee3e-4212-89fc-c9a3d387a27e Co-authored-by: koide3 <31344317+koide3@users.noreply.github.com>

koide3 · 2026-05-01T09:27:21Z

@copilot review this PR

Copilot · 2026-05-01T09:28:55Z

@copilot review this PR

The PR is in good shape. All six previously flagged issues have been addressed:

glk::ThinLines constructors — line_width (default 1.0f) added consistently across every overload in the header and implementation.
pyplot-like Python constructor (src/python/glk.cpp) — c.size() is now included in the max/min size computation; colors[0] access is guarded by size > 0.
async_light_viewer_context.cpp null-pointer UB — colors_buffer and indices_buffer are only populated when the input pointers are non-null, and nullptr is passed explicitly when the buffers are empty.
Python guik.cpp bindings — nullptr passed for empty colors/indices rather than relying on .data() of an empty container.
Docs — @param line_width added to update_thin_lines in light_viewer_context.hpp.

One minor observation: in async_light_viewer_context.cpp the ThinLines object is constructed with the default line_width and then set_line_width(line_width) is called right after. It would be marginally cleaner to pass line_width directly to the constructor (removing the extra call), but it's functionally equivalent and not a correctness issue.

add line_width to ThinLines interfaces

22ee0e3

koide3 requested a review from Copilot May 1, 2026 06:25

Copilot started reviewing on behalf of koide3 May 1, 2026 06:25 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Potential fix for pull request finding

744357a

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of koide3 May 1, 2026 07:58 View session

Add @param line_width doc entry to update_thin_lines in light_viewer_…

3d84c44

…context.hpp Agent-Logs-Url: https://github.com/koide3/iridescence/sessions/44514c31-be2a-4517-b17d-4363bfaa0973 Co-authored-by: koide3 <31344317+koide3@users.noreply.github.com>

Copilot finished work on behalf of koide3 May 1, 2026 07:59

Copilot started work on behalf of koide3 May 1, 2026 07:59 View session

Fix null pointer UB and empty vector .data() issues in async_light_vi…

64f5904

…ewer_context.cpp Agent-Logs-Url: https://github.com/koide3/iridescence/sessions/9dde4cd6-e065-42bf-a21c-0bfde09f1d09 Co-authored-by: koide3 <31344317+koide3@users.noreply.github.com>

Copilot finished work on behalf of koide3 May 1, 2026 08:00

Potential fix for pull request finding

219f03c

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of koide3 May 1, 2026 09:26 View session

Fix OOB access in pyplot-like ThinLines constructor when x/y/z empty …

8929991

…but c non-empty Agent-Logs-Url: https://github.com/koide3/iridescence/sessions/5408ca4b-ee3e-4212-89fc-c9a3d387a27e Co-authored-by: koide3 <31344317+koide3@users.noreply.github.com>

Copilot finished work on behalf of koide3 May 1, 2026 09:27

Copilot started work on behalf of koide3 May 1, 2026 09:27 View session

Copilot finished work on behalf of koide3 May 1, 2026 09:29

koide3 merged commit 1260ee2 into master May 1, 2026
1 check passed

koide3 deleted the line_width branch May 1, 2026 09:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add line_width to ThinLines interfaces#201

add line_width to ThinLines interfaces#201
koide3 merged 6 commits into
masterfrom
line_width

koide3 commented May 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

koide3 May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Uh oh!

Uh oh!

koide3 May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

koide3 May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

koide3 May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

koide3 commented May 1, 2026

Uh oh!

Copilot AI commented May 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

koide3 commented May 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

koide3 May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

koide3 May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

koide3 May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

koide3 May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

koide3 commented May 1, 2026

Uh oh!

Copilot AI commented May 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants