Clément Foucault fclem
  • I'm pixel pusher.

  • Joined on 2014-07-09
Clément Foucault commented on pull request blender/blender#105642 2023-03-15 12:54:06 +01:00
GPU: Refactor texture samplers

Remaining value.

Clément Foucault commented on pull request blender/blender#105642 2023-03-15 12:54:06 +01:00
GPU: Refactor texture samplers

The thing is, we already use 3D textures, and some of them use CLAMP_TO_BORDER (ex: Volumes Attributes) and some use EXTEND (ex: EEVEE Volume temp textures). If the choice to omit wrapping_z is to save a dimension, it should be documented. Maybe rename wrapping_y to wrapping_yz in this case and make Z follow Y extend mode.

Clément Foucault approved blender/blender#105770 2023-03-14 21:02:24 +01:00
Fix #105661: (Regression) Materials can use fewer images than before

The patch is fine. Just a note about codestyle.

Clément Foucault commented on pull request blender/blender#105770 2023-03-14 21:02:23 +01:00
Fix #105661: (Regression) Materials can use fewer images than before

Style: Avoid nesting as much as possible.

Clément Foucault commented on pull request blender/blender#105642 2023-03-12 17:57:25 +01:00
GPU: Refactor texture samplers

Rename to CLAMP_TO_BORDER instead of CLIP.

Clément Foucault commented on pull request blender/blender#105642 2023-03-12 17:57:25 +01:00
GPU: Refactor texture samplers

You are missing the wrapping_z for 3D texture.

Clément Foucault commented on pull request blender/blender#105642 2023-03-12 17:57:24 +01:00
GPU: Refactor texture samplers

Wrap is a bit misleading terminology here. So maybe GPUSamplerExtendMode?

Clément Foucault commented on pull request blender/blender#105642 2023-03-12 17:57:24 +01:00
GPU: Refactor texture samplers

It seems you did not took into account the clamp to border color case in all of these.

Clément Foucault commented on pull request blender/blender#105642 2023-03-12 10:06:07 +01:00
GPU: Refactor texture samplers

I still have to review all the usages. But the overall approach looks good.

Clément Foucault commented on pull request blender/blender#105642 2023-03-12 10:04:50 +01:00
GPU: Refactor texture samplers

The thing is, filtering is too generic of a name. I don't know if this will set the whole enum value or just one option if I see set_filtering(GPU_SAMPLER_FILTERING_MIPMAP). And I don't…

Clément Foucault opened issue blender/blender#105673 2023-03-11 21:38:12 +01:00
Screen Space Effects
Clément Foucault opened issue blender/blender#105672 2023-03-11 21:32:54 +01:00
EEVEE-Next Volume Rendering
Clément Foucault commented on pull request blender/blender#105642 2023-03-11 21:07:06 +01:00
GPU: Refactor texture samplers

I think name of the function is confusing.

Clément Foucault commented on pull request blender/blender#105642 2023-03-11 21:07:06 +01:00
GPU: Refactor texture samplers

These should be documented. For example, I'm not sure what an internal_sampler is right away.

Clément Foucault commented on pull request blender/blender#105642 2023-03-11 18:43:52 +01:00
GPU: Refactor texture samplers

I think you can omit the actual value set after the first one. Keep = 0 but remove = 1, = 2, ...

Clément Foucault commented on pull request blender/blender#105642 2023-03-11 18:43:52 +01:00
GPU: Refactor texture samplers

It was discussed that these counts should be defined like this:

Clément Foucault commented on pull request blender/blender#105642 2023-03-11 18:43:52 +01:00
GPU: Refactor texture samplers

All these left over lines can be remove.

Clément Foucault commented on pull request blender/blender#105642 2023-03-11 18:43:52 +01:00
GPU: Refactor texture samplers

Comment style.

Clément Foucault commented on pull request blender/blender#105642 2023-03-11 18:43:52 +01:00
GPU: Refactor texture samplers

I think it is a good opportunity to remove the e prefix which is not part of the code-style anymore.

Clément Foucault opened issue blender/blender#105649 2023-03-10 23:25:00 +01:00
EEVEE-Next Reflection Cubemaps