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.

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

Re-implement grow from start refinement algorithm #224

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from

Conversation

saxbophone
Copy link
Owner

@saxbophone saxbophone commented Nov 14, 2018

This PR adds a new figure refinement method, a re-implementation of the original method from v0.27.0 of libsxbp, which tries extending all lines from length 1 until the shape doesn't collide.

This PR is not complete! The heavily commented sections detail the remaining work that needs to be done.

Broadly speaking, the remaining work involves completely implementing the optimisation code (currently written but unused) which allows smarter line extensions to be made in certain collision situations. This is currently blocked pending a correction of an oversight in one of its dependent functions.

The function that needs correcting is sxbp_suggest_previous_length_callback(). Once this is done, then sxbp_suggest_previous_length() can be updated to use the actual value returned by sxbp_resolve_collision(), which it currently ignores.

@saxbophone saxbophone force-pushed the josh/217-re-implement-grow-from-start branch from 2f66c54 to e04bc80 Compare November 19, 2018 22:45
@saxbophone saxbophone force-pushed the josh/217-re-implement-grow-from-start branch from e04bc80 to 8165ca5 Compare December 11, 2018 21:17
…igure_collides_with()

I've hit a block: sxbp_walk_figure() currently doesn't return the line ID or pointer to the line that it's on -we kind of need this.
I will add this to sxbp_walk_figure(), as it is an internal-only private-use function.
Now using this in sxbp_figure_collides_with()
Also added in a check in the walk() callback to make it stop once it plots the last line we want it to plot so far
Remove silencing of unused-parameter warnings in function
This isn't really working out, so I'm going to manually unwind this recursion iteratively, using the original implementation in solve.c for guidance
… sxbp_figure_collides_with_callback()

For some reason, on line 155, we only have to quit when the line ID is GREATER than the max line requested
This confused me a lot, until I realised that otherwise it was only ever plotting the first bit of the last line, so never catching collisions properly!
Once I figured out the source of my woes and corrected it, I've since been able to refactor that function nicer
Have achieved the latter by mutilating the sxbp_resolve_collision() method a little bit, but it's going to refactored anyway.
@saxbophone saxbophone force-pushed the josh/217-re-implement-grow-from-start branch from 127257d to caba146 Compare December 13, 2018 03:30
sxbp_line_t* line,
sxbp_co_ord_t location,
void* callback_data
),
Copy link
Owner Author

Choose a reason for hiding this comment

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

The woes experienced in sxbp_suggest_previous_length_callback() can be better alleviated by adding an additional parameter to this function which will instruct the function on whether it needs to call the callback for every single-unit step of the figure's line (i.e. every single pixel when plotting a bitmap), or if it need only call the callback for the vertices of the figure's line (i.e. every time there is a direction change). This will make it much easier for the origin coördinates of a line of a given index to be found and therefore help fix the aforementioned function. This will also allow for more optimised SVG images to be produced by the library, as an SVG rendering need only know the vertices of the figure's line, however currently a separate line segment for each 'pixel' is generated, which is very inefficient and redundant SVG.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant