Compositor: Unify Texture node between CPU and GPU #118377

Merged

Omar Emara merged 2 commits from OmarEmaraDev/blender:unify-texture-node into main

2024-02-16 16:21:58 +01:00

Omar Emara commented

2024-02-16 13:40:07 +01:00

Member

The CPU and GPU implementations of the compositor Texture node sample
the texture at different locations, producing differences. This is
primarly an issue with half pixel offsets to evaluate the texture at the
center of the pixels.

The CPU compositor only adds the offsets if interpolation is disabled,
regardless if the texture is of type image or not. Further the offset
was wrong, since it was not a half pixel offset, but a full pixel one.

The offsets should always be added, since it won't make a difference in
the non interpolated case, and it should always be added especially if
interpolation is enabled. A comment in the old code mentions something
about artifacts, but it is possible that this is a consequence of the
wrong offset that was used.

This patch always adds a half pixel offset, following the GPU code.

The CPU and GPU implementations of the compositor Texture node sample the texture at different locations, producing differences. This is primarly an issue with half pixel offsets to evaluate the texture at the center of the pixels. The CPU compositor only adds the offsets if interpolation is disabled, regardless if the texture is of type image or not. Further the offset was wrong, since it was not a half pixel offset, but a full pixel one. The offsets should always be added, since it won't make a difference in the non interpolated case, and it should always be added especially if interpolation is enabled. A comment in the old code mentions something about artifacts, but it is possible that this is a consequence of the wrong offset that was used. This patch always adds a half pixel offset, following the GPU code.

Omar Emara added the

labels 2024-02-16 13:40:07 +01:00

Omar Emara added 1 commit 2024-02-16 13:40:17 +01:00

2721523162 Compositor: Unify Texture node between CPU and GPU

The CPU and GPU implementations of the compositor Texture node sample
the texture at different locations, producing differences. This is
primarly an issue with half pixel offsets to evaluate the texture at the
center of the pixels.

The CPU compositor only adds the offsets if interpolation is disabled,
regardless if the texture is of type image or not. Further the offset
was wrong, since it was not a half pixel offset, but a full pixel one.

The offsets should always be added, since it won't make a difference in
the non interpolated case, and it should always be added especially if
interpolation is enabled. A comment in the old code mentions something
about artifacts, but it is possible that this is a consequence of the
wrong offset that was used.

This patch always adds a half pixel offset, following the GPU code.

Omar Emara requested review from Sergey Sharybin 2024-02-16 13:40:31 +01:00

Omar Emara commented

2024-02-16 13:45:46 +01:00

Author

Member

To double check, here is the old implementation, where we are differencing the image we get directly from an Image node and the one we get from an Image Texture node. Notice how it looks like a horizontal partial derivative, due to the full pixel offset that the old code introduces.

The new implementation has a zero difference.

To double check, here is the old implementation, where we are differencing the image we get directly from an Image node and the one we get from an Image Texture node. Notice how it looks like a horizontal partial derivative, due to the full pixel offset that the old code introduces. ![20240216-141955.png](/attachments/6d355adf-7846-4603-b44a-c46d8915745f) The new implementation has a zero difference.

20240216-141955.png

1.3 MiB

Sergey Sharybin requested changes 2024-02-16 15:23:47 +01:00

source/blender/compositor/operations/COM_TextureOperation.cc Outdated

						
				@ -179,2 +147,2 @@

				    else {

				      it.out[0] = it.out[1] = it.out[2] = it.out[3];

				    TexResult texture_result;

				    const int result_type = multitex_ext_safe(

Sergey Sharybin commented

2024-02-16 15:23:43 +01:00

Owner

The multitex_ext_safe explicitly disables texture nodes. Is there a specific reason for this? Otherwise it should be something between these lines

    const int result_type = multitex_ext(texture_,
                                         coordinates,
                                         nullptr,
                                         nullptr,
                                         0,
                                         &texture_result,
                                         WorkScheduler::current_thread_id(),
                                         pool_,
                                         scene_color_manage_,
                                         false);

The `multitex_ext_safe` explicitly disables texture nodes. Is there a specific reason for this? Otherwise it should be something between these lines ``` const int result_type = multitex_ext(texture_, coordinates, nullptr, nullptr, 0, &texture_result, WorkScheduler::current_thread_id(), pool_, scene_color_manage_, false); ```

Omar Emara commented

2024-02-16 15:29:34 +01:00

Author

Member

No specific reason, I will use multitex_ext.

No specific reason, I will use `multitex_ext`.

OmarEmaraDev marked this conversation as resolved

Omar Emara added 1 commit 2024-02-16 15:35:50 +01:00

1a72305904 Use multitex_ext

Sergey Sharybin approved these changes 2024-02-16 16:20:20 +01:00

Omar Emara merged commit 9ab72eae38 into main

2024-02-16 16:21:58 +01:00

Omar Emara referenced this issue from a commit

2024-02-16 16:21:59 +01:00

Compositor: Unify Texture node between CPU and GPU

Omar Emara deleted branch unify-texture-node

2024-02-16 16:22:01 +01:00

Jeroen Bakker referenced this issue from a commit

2024-02-19 08:10:14 +01:00

Compositor: Unify Texture node between CPU and GPU

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#118377

No description provided.