jabuwu / rusty_spine

Spine runtime for Rust (and wasm!) transpiled from the official C Runtime.
https://docs.rs/rusty_spine
Other
43 stars 4 forks source link

custom attachments #20

Closed brandon-reinhart closed 10 months ago

brandon-reinhart commented 11 months ago

In progress PR adding the ability to create custom attachments that can be assigned to slots.

May optionally include the ability to create custom attachment loaders.

brandon-reinhart commented 11 months ago

I am working on the interface to modify a region attachment after it has been created. The region attachment can be empty upon creation, but to render any visible contents, the user needs to set all fields and call update_region. This should be clear and not something you can mess up.

brandon-reinhart commented 11 months ago

spSlot_setAttachment: It appears that setting the attachment on the slot does not increment the attachment ref count.

This seems strange to me, as the pointer is in use and if an attachment is set on a skin the ref count is incremented. It seems to me that an attachment that is in use by a slot should have its refcount increment and dispose called when it is no longer in use, but spine c does not do this.

As a result, it is possible to set an attachment on a slot and then drop the attachment, causing an access violation.

brandon-reinhart commented 11 months ago

Another issue I am investigating is that calling Slot::set_attachment cause an immediate panic. Calling the underlying c::spSlot_setAttachment works fine.

Edit: This is due to the non-incrementing ref count. If the refcount is incremented by spSlot_setAttachment, Slot::set_attachment is reliable.

brandon-reinhart commented 11 months ago

ref: https://github.com/EsotericSoftware/spine-runtimes/pull/2399 ref: https://github.com/EsotericSoftware/spine-runtimes/issues/2398

brandon-reinhart commented 11 months ago

An alternative to making attachment accessors mutable is to place the create attachment convenience function in RegionAttachment and have it take the loader and props as arguments.

brandon-reinhart commented 11 months ago

Example of the current code:

        let attachment_loader = AttachmentLoader::new_atlas_loader(&atlas);

        let region_props = RegionProps {
            x: 0.0,
            y: 0.0,
            scale_x: 1.0,
            scale_y: 1.0,
            rotation: 0.0,
            width: 128.0,
            height: 128.0,
            color: Color::new_rgba(1.0, 1.0, 1.0, 1.0),
        };

        let attachment = attachment_loader
            .create_region_attachment(None, "test", "head", &region_props)
            .unwrap();

        unsafe {
            let mut slot = controller.skeleton.find_slot_mut("front-shin").unwrap();
            slot.set_attachment(Some(attachment.clone()));
        }
jabuwu commented 11 months ago

Nice work.

spSlot_setAttachment: It appears that setting the attachment on the slot does not increment the attachment ref count.

We shouldn't be making modifications directly to spine_c.rs, since it's auto-generated. We will need to wait for a response to the Spine C PRs.

brandon-reinhart commented 11 months ago

Nice work.

spSlot_setAttachment: It appears that setting the attachment on the slot does not increment the attachment ref count.

We shouldn't be making modifications directly to spine_c.rs, since it's auto-generated. We will need to wait for a response to the Spine C PRs.

Yeah, that's why I noted this is temporary. The goal of this was to prove out it resolves the problems, which it does in my testing.

brandon-reinhart commented 11 months ago

This is all usable. Anything else I should do here? It doesn't require any changes from the spine runtime.

brandon-reinhart commented 10 months ago

Ship it!