vE5li / korangar

A next-gen Ragnarok Online client
MIT License
147 stars 33 forks source link

Picker values can fully store 32-bit values. #112

Closed hasenbanck closed 2 weeks ago

hasenbanck commented 3 weeks ago

This removes the implicit reliance of an entity ID to not use the full 32-bit value range. Since we can't assume the actual range that is used inside the 32 bit of an EntityId, we need to properly encode the full 32 bit inside the picker buffer.

For this we use a Rg32Uint texture, to store the picker values. This doubles the picker texture size (4K texture is around 65 MiB). The runtime of the shader is still very low and insignificant compared to other render passes.

This also allows us to encode much more data into the picker value later, if we need to do so.

hasenbanck commented 2 weeks ago

Add changes based on your feedback:

  1. enum PickerValueType's value is not auto assigned. But I added a "Nothing" enum value to signal, that nothing was selected with the mouse. It's assigned on fall through, so everything that isn't the other enum values, is automatically "Nothing".
  2. Removed magic constant by setting it with the help of a pipeline constant.
  3. Added funky linter warning by replacing a cast with a concrete conversion to u64.
  4. Updated tests.

Feel free to change stuff you don't like.

hasenbanck commented 2 weeks ago

Still thinking about the clear value. We could do the following when creating a new render pass for the PickerRenderTarget:

        let (clear_high, clear_low) = <(u32, u32)>::from(PickerTarget::Nothing);
        let clear_color = wgpu::Color {
            r: f64::from(clear_high),
            g: f64::from(clear_low),
            b: 0.0,
            a: 0.0,
        };

Edit: Uploaded a version with this idea. The branch currently has no magic constants left and everything is based on the automatic enum values.

vE5li commented 2 weeks ago

Very nice, I really like this :+1: