blender
/
blender
136
418
Fork 600
Code Issues 6.3k Pull Requests 779 Projects 19 Wiki Activity

Realtime Compositor: Implement Movie Distortion node #108230

Merged
Omar Emara merged 3 commits from OmarEmaraDev/blender:compositor-movie-distortion into main 2023-06-07 14:45:53 +02:00
Conversation 1 Commits 3 Files Changed 9 +39 -21
5 changed files with 39 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 42a0070f58 - Show all commits

13
source/blender/blenkernel/BKE_tracking.h
View File

@ -21,6 +21,7 @@ struct MovieClipUser;
struct MovieDistortion;
struct MovieReconstructContext;
struct MovieTracking;
struct MovieTrackingCamera;
struct MovieTrackingMarker;
struct MovieTrackingObject;
struct MovieTrackingPlaneMarker;
@ -464,9 +465,15 @@ void BKE_tracking_camera_principal_point_pixel_get(struct MovieClip *clip,
void BKE_tracking_camera_principal_point_pixel_set(struct MovieClip *clip,
const float principal_point_pixel[2]);
bool BKE_tracking_camera_equal(const struct MovieTrackingCamera *a,
const struct MovieTrackingCamera *b);
uint64_t BKE_tracking_camera_hash(const struct MovieTrackingCamera *camera);
/* Compares distortion related parameters of camera. Ideally, this implementation will be
* abstracted away in the future, but for now, one needs to be careful about it and handle any
* extra parameters of distortions models. */
bool BKE_tracking_camera_distortion_equal(const struct MovieTrackingCamera *a,
const struct MovieTrackingCamera *b);
/* Hashes distortion related paramaters of camera. Ideally, this implementation will be
* abstracted away in the future, but for now, one needs to be careful about it and handle any
* extra parameters of distortions models. */
uint64_t BKE_tracking_camera_distortion_hash(const struct MovieTrackingCamera *camera);
/* --------------------------------------------------------------------
* (Un)distortion.

28
source/blender/blenkernel/intern/tracking.cc
View File

@ -2201,7 +2201,8 @@ void BKE_tracking_camera_principal_point_pixel_set(MovieClip *clip,
principal_point_pixel, frame_width, frame_height, camera->principal_point);
}
bool BKE_tracking_camera_equal(const MovieTrackingCamera *a, const MovieTrackingCamera *b)
bool BKE_tracking_camera_distortion_equal(const MovieTrackingCamera *a,
const MovieTrackingCamera *b)
OmarEmaraDev marked this conversation as resolved Outdated
Sergey Sharybin commented 2023-05-30 10:47:17 +02:00
Outdated
Review
Copy Link

This is a bit confusing. Camera also defines sensor width, which is not compared here.

This is a bit confusing. Camera also defines sensor width, which is not compared here.
{
if (a->pixel_aspect != b->pixel_aspect || a->focal != b->focal ||
!equals_v2v2(a->principal_point, b->principal_point))
@ -2224,38 +2225,41 @@ bool BKE_tracking_camera_equal(const MovieTrackingCamera *a, const MovieTracking
return a->brown_k1 == b->brown_k1 && a->brown_k2 == b->brown_k2 &&
a->brown_k3 == b->brown_k3 && a->brown_k4 == b->brown_k4 &&
a->brown_p1 == b->brown_p1 && a->brown_p2 == b->brown_p2;
default:
BLI_assert_unreachable();
return false;
}
OmarEmaraDev marked this conversation as resolved
Sergey Sharybin commented 2023-05-30 09:24:21 +02:00
Review
Copy Link

Please do not add default unless absolutely needed, as it makes it very hard to see at compile time where enumerator value is forgotten to be checked when adding a new value.

In this example it is as easy as moving the BLI_assert_unreachable past the switch statement.

Please do not add `default` unless absolutely needed, as it makes it very hard to see at compile time where enumerator value is forgotten to be checked when adding a new value. In this example it is as easy as moving the `BLI_assert_unreachable` past the `switch` statement.
BLI_assert_unreachable();
return false;
}
uint64_t BKE_tracking_camera_hash(const MovieTrackingCamera *camera)
uint64_t BKE_tracking_camera_distortion_hash(const MovieTrackingCamera *camera)
{
using namespace blender;
switch (camera->distortion_model) {
OmarEmaraDev marked this conversation as resolved
Sergey Sharybin commented 2023-05-30 10:46:13 +02:00
Review
Copy Link

Is it intended to skip distortion_model, focal and so on?

Is it intended to skip `distortion_model`, `focal` and so on?
case TRACKING_DISTORTION_MODEL_POLYNOMIAL:
return get_default_hash_3(float2(camera->pixel_aspect, camera->focal),
return get_default_hash_4(camera->distortion_model,
float2(camera->pixel_aspect, camera->focal),
float2(camera->principal_point),
float3(camera->k1, camera->k2, camera->k3));
case TRACKING_DISTORTION_MODEL_DIVISION:
return get_default_hash_3(float2(camera->pixel_aspect, camera->focal),
return get_default_hash_4(camera->distortion_model,
float2(camera->pixel_aspect, camera->focal),
float2(camera->principal_point),
float2(camera->division_k1, camera->division_k2));
case TRACKING_DISTORTION_MODEL_NUKE:
return get_default_hash_3(float2(camera->pixel_aspect, camera->focal),
return get_default_hash_4(camera->distortion_model,
float2(camera->pixel_aspect, camera->focal),
float2(camera->principal_point),
float2(camera->nuke_k1, camera->nuke_k2));
case TRACKING_DISTORTION_MODEL_BROWN:
return get_default_hash_4(
float2(camera->pixel_aspect, camera->focal),
float2(camera->principal_point),
OmarEmaraDev marked this conversation as resolved
Sergey Sharybin commented 2023-05-30 10:43:51 +02:00
Review
Copy Link

Same as above.

Same as above.
float4(camera->brown_k1 && camera->brown_k2 && camera->brown_k3 && camera->brown_k4),
float4(camera->brown_k1, camera->brown_k2, camera->brown_k3, camera->brown_k4),
float2(camera->brown_p1, camera->brown_p2));
default:
BLI_assert_unreachable();
return 0;
}
BLI_assert_unreachable();
return 0;
}
/* --------------------------------------------------------------------
* (Un)distortion.

4
source/blender/compositor/realtime_compositor/cached_resources/COM_distortion_grid.hh
View File

@ -1,4 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
/* SPDX-FileCopyrightText: 2023 Blender Foundation
OmarEmaraDev marked this conversation as resolved Outdated
Sergey Sharybin commented 2023-05-30 09:25:02 +02:00
Outdated
Review
Copy Link

Please add copyright to the new files.

Please add copyright to the new files.
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#pragma once

11
source/blender/compositor/realtime_compositor/cached_resources/intern/distortion_grid.cc
View File

@ -1,4 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
/* SPDX-FileCopyrightText: 2023 Blender Foundation
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#include <cstdint>
#include <memory>
@ -36,13 +38,14 @@ DistortionGridKey::DistortionGridKey(MovieTrackingCamera camera,
uint64_t DistortionGridKey::hash() const
{
return get_default_hash_4(BKE_tracking_camera_hash(&camera), size, type, calibration_size);
return get_default_hash_4(
BKE_tracking_camera_distortion_hash(&camera), size, type, calibration_size);
}
bool operator==(const DistortionGridKey &a, const DistortionGridKey &b)
{
return BKE_tracking_camera_equal(&a.camera, &b.camera) && a.size == b.size && a.type == b.type &&
a.calibration_size == b.calibration_size;
return BKE_tracking_camera_distortion_equal(&a.camera, &b.camera) && a.size == b.size &&
a.type == b.type && a.calibration_size == b.calibration_size;
}
/* --------------------------------------------------------------------

4
source/blender/compositor/realtime_compositor/shaders/infos/compositor_movie_distortion_info.hh
View File

@ -1,4 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
/* SPDX-FileCopyrightText: 2023 Blender Foundation
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#include "gpu_shader_create_info.hh"