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

Automatically resize grid according to the model size #12

Open
Peque opened this issue Sep 29, 2019 · 11 comments
Open

Automatically resize grid according to the model size #12

Peque opened this issue Sep 29, 2019 · 11 comments
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed

Comments

@Peque
Copy link
Collaborator

Peque commented Sep 29, 2019

Right now the grid is defined with its size and number of divisions.

While this is fine and should be kept as a possibility, the default should set a grid with a size double the longest dimension of the displayed model.

@Peque Peque added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers hacktoberfest labels Sep 29, 2019
@garrlker
Copy link

garrlker commented Oct 2, 2019

I'd like to try my hand at this!

@Peque
Copy link
Collaborator Author

Peque commented Oct 2, 2019

Great! 😊

@shimwell
Copy link
Contributor

Perhaps it would also be nice to have the option to not include the grid at all

@Peque
Copy link
Collaborator Author

Peque commented May 14, 2021

@shimwell Yeah! 👍

@shimwell
Copy link
Contributor

Does this approach sound ok to you @Peque ?

make another variable called raw_gridless_html_template which is the same as raw_html_template but misses the parent.gridsize and griddivisions parts

then add an if statement to the run function and if gridsize and griddivisions are not specified in the user options then make use of the raw_gridless_html_template

@Peque
Copy link
Collaborator Author

Peque commented May 15, 2021

@shimwell So I guess you are suggesting that, by default, no grid should be displayed (unless the user defines gridsize).

I think I would rather leave the default behavior as-is (i.e.: show a grid).

So, how could we disable the grid? Instead of adding a new parameter, I'd say gridsize=0 should disable the grid. Maybe an if in the main.js:init() function to check if gridsize is greater than 0?

@shimwell
Copy link
Contributor

shimwell commented May 16, 2021

That sounds better

@shimwell
Copy link
Contributor

Changing this

https://github.com/Peque/sphinxcadquery/blob/917dedfe0dd7a4aae092922b1a3c0f323c099dc4/sphinxcadquery/sphinxcadquerystatic/main.js#L143

to this appears to work for me

        if ( gsize > 0 ){
            var grid = new TranslucentGrid( gsize, gdivs, 0x888888, 0xdddddd, 0.6, );
        }

@Peque
Copy link
Collaborator Author

Peque commented May 16, 2021

@shimwell What about the references to grid just bellow? Don't they throw an error?

https://github.com/Peque/sphinxcadquery/blob/917dedfe0dd7a4aae092922b1a3c0f323c099dc4/sphinxcadquery/sphinxcadquerystatic/main.js#L145-L147

Maybe put all those lines inside the if clause? (from the var grid declaration to the scene.add(grid) call).

@shimwell
Copy link
Contributor

shimwell commented May 16, 2021

Yep you are right. I was just testing it worked with out the grid

I can make a PR for your consideration

@shimwell
Copy link
Contributor

Just to get this issue back on track I wanted to mention that Cadquery has a bounding box that can be used to find the shape size in x,y,z https://cadquery.readthedocs.io/en/latest/classreference.html?highlight=boundingbox#cadquery.Shape.BoundingBox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants