Ensure grayscale angle shading renders in 2D view#77
Ensure grayscale angle shading renders in 2D view#77kryptokommunist wants to merge 1 commit intomainfrom
Conversation
Summary of ChangesHello @kryptokommunist, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new visual feature to the Doyle Spiral Studio, enabling users to render 2D regions with grayscale shading based on the angle of the lines. This enhancement provides an alternative visualization mode, allowing for a different perspective on the generated spirals by mapping line angles to a black-to-white gradient. The changes span the UI, application defaults, and the core rendering engine to seamlessly integrate this new functionality. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully introduces a new feature for grayscale angle shading in the 2D view. The changes are well-structured, adding a UI toggle in the HTML, passing the new shade_by_angle parameter through the JavaScript application layers, and implementing the core rendering logic in doyle_spiral_engine.js. The implementation of the solid fill based on angle is clean. I have one suggestion to improve the robustness of how the representative angle of an arc group is determined, which will lead to more predictable coloring. Overall, this is a solid contribution.
| continue; | ||
| } | ||
| const ringIdx = group.ringIndex ?? 0; | ||
| const referenceArc = group.arcs.find(arc => arc?.start && arc?.end); |
There was a problem hiding this comment.
The current implementation uses find to select the first valid arc from the group to determine the angle for shading. This might not be representative of the group's overall orientation, especially for complex groups composed of multiple arcs with different lengths and orientations (e.g., after _extendGroupsWithNeighbours runs). A more robust approach would be to use the longest arc in the group, as its orientation is more likely to be representative. This will lead to more stable and predictable coloring.
| const referenceArc = group.arcs.find(arc => arc?.start && arc?.end); | |
| const referenceArc = group.arcs.filter(arc => arc?.start && arc?.end).sort((a, b) => Math.hypot(b.end.re - b.start.re, b.end.im - b.start.im) - Math.hypot(a.end.re - a.start.re, a.end.im - a.start.im))[0]; |
Summary
Testing
Codex Task