Fix: qSVG polygonize - polygons creation condition#44
Open
mislavivanda wants to merge 1 commit intoekymo:masterfrom
Open
Fix: qSVG polygonize - polygons creation condition#44mislavivanda wants to merge 1 commit intoekymo:masterfrom
mislavivanda wants to merge 1 commit intoekymo:masterfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🐞Current Issue
A condition in the
qSVG.polygonize()method always resolves totruebecause it compares an array(realCoords.outside) with a number(realCoords.inside.length). This leads to incorrect handling of detected polygons.✅Test Steps
masterbranch🔎You should see a yellow-highlighted room area. Currently, its edges run through the center of the walls.
✅Our intended behavior is for the room area to match the interior edges of the walls. This commit fixes that.
🛠️
polygonize()Method OverviewThe goal of the
polygonize()method is to detect polygonal regions by analyzing wall segments and constructing a graph in which it detects closed loops. These loops are evaluated, and if they meet certain conditions (not marked withbypass = 1), they are pushed into thepolygonsarray as candidates for rooms.The condition in question is intended to verify whether the detected loop forms a proper room polygon. This is also hinted in the inline comment:
The check attempts to confirm that the loop is symmetrical and well-formed by comparing the
insideandoutsidecoordinate sets. If both arrays have the same length, it suggests a clean, closed shape, andrealCoords.insidecan be trusted.If they don't match, the method still includes the shape, but uses a fallback approach by using
coordsarray. These fallback polygons can still be useful, but are considered less reliable with lower confidence and precision compared to the properly closed loops. They can be interpreted as approximated closed shapes that might not perfectly align with actual room boundaries.✅Proposed fix
Update the condition to correctly compare the lengths of the
insideandoutsidearrays:This simple fix aligns with the original intent, and ensures proper validation of real, symmetrical room polygons.
We all make typos - it's part of writing software!