guillaumechereau / goxel

Goxel: Free and Open Source 3D Voxel Editor
GNU General Public License v3.0
2.72k stars 219 forks source link

Wrap voxels in layer/selection #363

Closed madd-games closed 3 months ago

madd-games commented 3 months ago

This PR adds a "wrap" feature. NOTE: Please check the French translation of "wrap" because it's almost certainly incorrect.

In the Layers panel, the wrap box allows the user to wrap voxels on the whole layer (independently of other layers). In the 'Selection' tool, the wrap box allows the user to wrap the voxels in the selection box, across all layers.

This basically means moving the voxels in a direction along one of the axis, and when the voxels reach a certain boundary, they're wrapped to the opposite boundary. The easieest way to see it is: select a box, fill it with a color, paint a different color on some random voxels on each side, then press the wrap buttons and see the voxels move (or just read the code I guess).

I found this to be very useful when working with tileable voxel blocks. GIMP has a similar feature and it's often used for removing seams between tiles.

guillaumechereau commented 3 months ago

Thanks for the PR! I tried and it seems to work well. I am not sure if this should be added like that in the UI though, since this is not a common operation and I am trying to keep the UI clean.

The best in my opinion would be to have a menu: Filters -> Wrap, that would open a new panel with the wrap UI. This is actually something that I added a long time ago (with a 'grow' filter) and removed after a while because I thought it wasn't very good but maybe it's time to put this back. It would a good place to add other common filter effects too.

Do you think you can modify the PR so that there is a single 'wrap' UI, as a new panel for now, with a dummy icon?

guillaumechereau commented 3 months ago

Thinking about it, another way would be to just make 'wrap' an other tool, with a dedicated icon.

madd-games commented 3 months ago

A menu might not be a good place for it, because usually i want to repeatedly click the button, to move a few voxels over; so I'd like it to be a button that I can easily click. I re-read and I see that you only suggested that the menu opens a new panel, not that all the options are in the menu. Either way, the below question still applies.

Could you clarify your suggestion for the separate panel? Right now the box is used in two locations: the layers panel and the selection panel, and each applies the wrapping to different aspects (layer or selection). I think if we move it to a separate panel, it would be confusing which it applies to.

I think the same problem might occur with a separate tool: how would we indicate whether the wrapping is applied to the layer or the selection? And would we have to switch between selection and wrap tools if we want to change the selection?

Another way to keep it cleaner might be to hide the 'wrap box' inside an expandable panel, in each of the locations.

I'm happy to make more changes to the PR, once we figure out what we want

guillaumechereau commented 3 months ago

I was thinking that the menu would open a window with the wrap UI, that would stay open and could be moved around and stay open while we use other tools.

If needed we could add a checkbox to decide if we wrap to the selection or layer (or even image), but in my opinion this should all be implicit: if there is a selection we use it, otherwise if the layer is bounded we use that, otherwise if the image is bounded we use that.

madd-games commented 3 months ago

Okay, I now understand your suggestion and I think the implicit behaviour is a better idea.

Is there any example in the code where such a movable panel is currently being open? If there is it would help me to figure out how to implement this.

Or would you prefer if it's a separate tool instead?

In any case I'll most likely implement the changes at some point today, if not then tomorrow.

guillaumechereau commented 3 months ago

Not at the moment. Originally I used a simple modal popup but now that we support moving windows I would like to use that. I am thinking of doing something similar to the tool struct for filters (or effects?) maybe, with automatic registrations. That way it will be easy to add more of them. One I had in mind is a hue/saturation/value filter.

It’s late here and I probably won’t have time tomorrow but I’ll try to draft something if I have time.

On Wed, Mar 20, 2024 at 11:02 PM Madd Games @.***> wrote:

Okay, I now understand your suggestion and I think the implicit behaviour is a better idea.

Is there any example in the code where such a movable panel is currently being open? If there is it would help me to figure out how to implement this.

In any case I'll most likely implement the changes at some point today, if not then tomorrow.

— Reply to this email directly, view it on GitHub https://github.com/guillaumechereau/goxel/pull/363#issuecomment-2009786338, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2JH3D3NIJH5DTTXX6JNDYZGQJRAVCNFSM6AAAAABE7QOUA2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBZG44DMMZTHA . You are receiving this because you commented.Message ID: @.***>

madd-games commented 3 months ago

Alright, I guess for now I'll add it as a tool (when i get time today or tomorrow). This will allow me to add the implicit behaviours too.

guillaumechereau commented 3 months ago

OK I pushed the inital support for 'Filters', that will hopefully allow to add all sort of effects without having to clutter the tools UI. You can have a look at the 'dummy' filter I added as an example for now, and see if you can make a new 'Wrap' filter from that.

madd-games commented 3 months ago

Thanks, I'll look at it soon, i'll let you know when the branch is ready.

madd-games commented 3 months ago

It is ready now.

guillaumechereau commented 3 months ago

Thanks. I merged.