Skip to content

Commit 8789da8

Browse files
EmilDohneEmilDohne
committed
Fix test failures
1 parent 6fc74fc commit 8789da8

File tree

9 files changed

+54
-53
lines changed

9 files changed

+54
-53
lines changed

‎PhotoshopAPI/src/Core/Render/Composite.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ namespace Composite
183183

184184
// Compute the intersection of the canvas and the layer as the layer may go outside of the canvas' bbox.
185185
auto canvas_bbox = Geometry::BoundingBox<int>(Geometry::Point2D<int>(0, 0), Geometry::Point2D<int>(canvas.width, canvas.height));
186-
auto layer_bbox = layer.bbox(canvas.width, canvas.height);
186+
auto layer_bbox = layer.bbox();
187187
auto _intersected_bbox = Geometry::BoundingBox<int>::intersect(canvas_bbox, layer_bbox);
188188
if (!_intersected_bbox)
189189
{

‎PhotoshopAPI/src/Core/Render/ImageBuffer.h

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -92,16 +92,11 @@ namespace Render
9292
}
9393

9494
/// Compute the bounding box of the channel based on the document.
95-
Geometry::BoundingBox<int> bbox(size_t document_width, size_t document_height) const
95+
Geometry::BoundingBox<int> bbox() const
9696
{
97-
// Get the canvas center as Point2D so we can offset the bbox by the positions
98-
auto center = Geometry::Point2D<int>(
99-
static_cast<int>(std::round(static_cast<double>(document_width) / 2)),
100-
static_cast<int>(std::round(static_cast<double>(document_height) / 2))
101-
);
102-
103-
auto absolute_position = center + this->position;
97+
auto absolute_position = this->position;
10498
auto _bbox = Geometry::BoundingBox<int>(Geometry::Point2D<int>(0, 0), Geometry::Point2D<int>(this->width, this->height));
99+
_bbox.offset(-_bbox.center());
105100
_bbox.offset(absolute_position);
106101

107102
return _bbox;
@@ -841,8 +836,8 @@ namespace Render
841836
if (this->mask)
842837
{
843838
const auto _mask = this->mask.value();
844-
auto _alpha_bbox = this->bbox(document_width, document_height);
845-
auto _mask_bbox = this->mask.value().bbox(document_width, document_height);
839+
auto _alpha_bbox = this->bbox();
840+
auto _mask_bbox = this->mask.value().bbox();
846841

847842
// create the intersection bbox and compute the min and max values
848843
auto _roi_bbox = Geometry::BoundingBox<int>::intersect(_alpha_bbox, _mask_bbox);
@@ -933,14 +928,8 @@ namespace Render
933928
constexpr int mask_index() { return -2; }
934929

935930
/// Compute the bounding box of the layer taking into account the mask (if present)
936-
Geometry::BoundingBox<int> bbox(size_t document_width, size_t document_height) const
931+
Geometry::BoundingBox<int> bbox() const
937932
{
938-
// Get the canvas center as Point2D so we can offset the bbox by the positions
939-
auto center = Geometry::Point2D<int>(
940-
static_cast<int>(std::round(static_cast<double>(document_width) / 2)),
941-
static_cast<int>(std::round(static_cast<double>(document_height) / 2))
942-
);
943-
944933
uint8_t mask_default = this->metadata.mask_default_value.value_or(0);
945934
std::optional<Geometry::BoundingBox<int>> mask_bbox;
946935

@@ -949,14 +938,14 @@ namespace Render
949938
auto mask_width = this->mask.value().width;
950939
auto mask_height = this->mask.value().height;
951940

952-
auto absolute_mask_position = center + this->mask.value().position;
941+
auto absolute_mask_position = this->mask.value().position;
953942

954943
auto _bbox = Geometry::BoundingBox<int>(Geometry::Point2D<int>(0, 0), Geometry::Point2D<int>(mask_width, mask_height));
955944
_bbox.offset(absolute_mask_position);
956945
mask_bbox.emplace(std::move(_bbox));
957946
}
958947

959-
auto absolute_position = center + this->metadata.position;
948+
auto absolute_position = this->metadata.position;
960949

961950
// construct the bbox and center it about 0 so we can then offset it in place
962951
auto image_bbox = Geometry::BoundingBox<int>(Geometry::Point2D<int>(0, 0), Geometry::Point2D<int>(this->width, this->height));

‎PhotoshopAPI/src/LayeredFile/LayerTypes/ImageLayer.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ struct ImageLayer final : public Layer<T>, public WritableImageDataMixin<T>
6363

6464
// ---------------------------------------------------------------------------------------------------------------------
6565
// ---------------------------------------------------------------------------------------------------------------------
66-
void set_write_compression(Enum::Compression _compcode)
66+
void set_write_compression(Enum::Compression _compcode) override
6767
{
6868
for (const auto& [_, channel_ptr] : WritableImageDataMixin<T>::m_ImageData)
6969
{
@@ -145,7 +145,7 @@ struct ImageLayer final : public Layer<T>, public WritableImageDataMixin<T>
145145

146146
// ---------------------------------------------------------------------------------------------------------------------
147147
// ---------------------------------------------------------------------------------------------------------------------
148-
void set_image_data(const data_type& data, int32_t width, int32_t height)
148+
void set_image_data(const data_type& data, int32_t width, int32_t height) override
149149
{
150150
WritableImageDataMixin<T>::impl_set_image_data(
151151
data,
@@ -245,7 +245,7 @@ struct ImageLayer final : public Layer<T>, public WritableImageDataMixin<T>
245245

246246
// ---------------------------------------------------------------------------------------------------------------------
247247
// ---------------------------------------------------------------------------------------------------------------------
248-
void set_channel(Enum::ChannelIDInfo _id, const std::vector<T>& channel)
248+
void set_channel(Enum::ChannelIDInfo _id, const std::vector<T>& channel) override
249249
{
250250
WritableImageDataMixin<T>::impl_set_channel(
251251
WritableImageDataMixin<T>::idinfo_from_variant(_id, Layer<T>::m_ColorMode),
@@ -371,7 +371,7 @@ struct ImageLayer final : public Layer<T>, public WritableImageDataMixin<T>
371371
auto& [key, channel_buffer] = pair;
372372
auto idinfo = Enum::toChannelIDInfo(key, Layer<T>::m_ColorMode);
373373
auto buffer_span = std::span<T>(channel_buffer.begin(), channel_buffer.end());
374-
WritableImageDataMixin<T>::m_ImageData[idinfo]->getData<T>(buffer_span, num_threads);
374+
WritableImageDataMixin<T>::m_ImageData[idinfo]->template getData<T>(buffer_span, num_threads);
375375
});
376376

377377
if (Layer<T>::has_mask())

‎PhotoshopAPI/src/LayeredFile/LayerTypes/MaskDataMixin.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ struct MaskMixin
3636
using channel_type = std::unique_ptr<ImageChannel>;
3737

3838
/// Colormode independent mask index as Enum::ChannelIDInfo that may be used
39-
static constexpr auto s_mask_index = Enum::ChannelIDInfo{ Enum::ChannelID::RealUserSuppliedLayerMask, -2 };
39+
static constexpr auto s_mask_index = Enum::ChannelIDInfo{ Enum::ChannelID::UserSuppliedLayerMask, -2 };
4040

4141
MaskMixin() = default;
4242
MaskMixin(

‎PhotoshopAPI/src/LayeredFile/LayerTypes/SmartObjectLayer.h

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include <fstream>
2121
#include <string>
22+
#include <cassert>
2223

2324
#include "fmt/core.h"
2425
#include <Eigen/Dense>
@@ -122,7 +123,7 @@ struct SmartObjectLayer final: public Layer<T>, public ImageDataMixin<T>
122123

123124
// ---------------------------------------------------------------------------------------------------------------------
124125
// ---------------------------------------------------------------------------------------------------------------------
125-
void set_write_compression(Enum::Compression _compcode)
126+
void set_write_compression(Enum::Compression _compcode) override
126127
{
127128
for (const auto& [_, channel_ptr] : ImageDataMixin<T>::m_ImageData)
128129
{
@@ -964,14 +965,16 @@ struct SmartObjectLayer final: public Layer<T>, public ImageDataMixin<T>
964965
{
965966
throw std::runtime_error(fmt::format("SmartObjectLayer '{}': Unexpected failure while evaluating the mesh: m_LinkedLayers is a nullptr", Layer<T>::m_LayerName));
966967
}
967-
968968
if (!is_mesh_cache_valid())
969969
{
970+
auto linked_layer = m_LinkedLayers->at(m_Hash);
971+
assert(linked_layer != nullptr);
972+
970973
// Get the warp mesh at a resolution of 20 pixels per subdiv. Ideally we'd lower this as we improve our algorithms
971974
auto warp_surface = m_SmartObjectWarp.surface();
972975
m_MeshCache = warp_surface.mesh(
973-
m_LinkedLayers->at(m_Hash)->width() / 20,
974-
m_LinkedLayers->at(m_Hash)->height() / 20,
976+
linked_layer->width() / 20,
977+
linked_layer->height() / 20,
975978
true // move_to_zero, that way we don't have to deal with bbox stuff
976979
);
977980
store_mesh_was_cached();
@@ -1004,7 +1007,7 @@ struct SmartObjectLayer final: public Layer<T>, public ImageDataMixin<T>
10041007
/// we recompute the image data and assign the warp to m_Warp.
10051008
// ---------------------------------------------------------------------------------------------------------------------
10061009
// ---------------------------------------------------------------------------------------------------------------------
1007-
data_type evaluate_image_data()
1010+
data_type evaluate_image_data() override
10081011
{
10091012
PSAPI_PROFILE_FUNCTION();
10101013
if (!m_LinkedLayers)
@@ -1086,7 +1089,12 @@ struct SmartObjectLayer final: public Layer<T>, public ImageDataMixin<T>
10861089
}
10871090
else if (idinfo == s_alpha_idinfo)
10881091
{
1089-
image_data = std::vector<T>(linked_layer->width() * linked_layer->height());
1092+
T value = std::numeric_limits<T>::max();
1093+
if constexpr (std::is_same_v<T, float32_t>)
1094+
{
1095+
value = 1.0f;
1096+
}
1097+
image_data = std::vector<T>(linked_layer->width() * linked_layer->height(), value);
10901098
}
10911099
else
10921100
{
@@ -1099,8 +1107,8 @@ struct SmartObjectLayer final: public Layer<T>, public ImageDataMixin<T>
10991107
Render::ConstChannelBuffer<T> orig_buffer(image_data, linked_layer->width(), linked_layer->height());
11001108

11011109
// Generate the warped result
1102-
std::vector<T> channel_warp(Layer<T>::width() * Layer<T>::height());
1103-
Render::ChannelBuffer<T> channel_warp_buffer(channel_warp, Layer<T>::width(), Layer<T>::height());
1110+
std::vector<T> channel_warp(this->width() * this->height());
1111+
Render::ChannelBuffer<T> channel_warp_buffer(channel_warp, this->width(), this->height());
11041112

11051113
auto& warp_mesh = this->evaluate_mesh_or_get_cached();
11061114

@@ -1123,7 +1131,7 @@ struct SmartObjectLayer final: public Layer<T>, public ImageDataMixin<T>
11231131
Layer<T>::m_CenterY
11241132
);
11251133
this->store_was_cached(idinfo);
1126-
return std::move(channel_warp);
1134+
return channel_warp;
11271135
}
11281136
};
11291137

‎PhotoshopAPI/src/LayeredFile/LayeredFile.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ struct LayeredFile
670670
/// Linked layers are external files associated with the layered file. In the context of
671671
/// e.g. SmartObjects these will hold the raw file bytes so that multiple smart objects
672672
/// can access the same layers without data duplication
673-
std::shared_ptr<LinkedLayers<T>> m_LinkedLayers;
673+
std::shared_ptr<LinkedLayers<T>> m_LinkedLayers = std::make_shared<LinkedLayers<T>>();
674674

675675

676676
std::vector<std::shared_ptr<Layer<T>>> generate_flattened_layers_impl(const LayerOrder order)

‎PhotoshopAPI/src/Util/CoordinateUtil.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ inline ChannelExtents generate_extents(const ChannelCoordinates coordinates)
6161
{
6262
ChannelExtents extents = {};
6363

64-
extents.top = static_cast<int32_t>(coordinates.centerY - .5 * coordinates.width);
65-
extents.left = static_cast<int32_t>(coordinates.centerX - .5 * coordinates.width);
64+
extents.top = static_cast<int32_t>(std::round(coordinates.centerY - .5f * coordinates.height));
65+
extents.left = static_cast<int32_t>(std::round(coordinates.centerX - .5f * coordinates.width));
6666
extents.bottom = extents.top + coordinates.height;
6767
extents.right = extents.left + coordinates.width;
6868

‎PhotoshopTest/src/TestImageLayer/TestImageLayer.cpp

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ TEST_CASE("Construct ImageLayer with int16_t ctor")
3333
.height = height,
3434
};
3535

36-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
36+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
3737

3838
CHECK(layer->width() == width);
3939
CHECK(layer->height() == height);
@@ -66,7 +66,7 @@ TEST_CASE("Construct ImageLayer with mask as part of image data")
6666
.height = height,
6767
};
6868

69-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
69+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
7070

7171
CHECK(layer->width() == width);
7272
CHECK(layer->height() == height);
@@ -99,7 +99,7 @@ TEST_CASE("Construct ImageLayer with explicit mask")
9999
.height = height,
100100
};
101101

102-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
102+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
103103

104104
CHECK(layer->width() == width);
105105
CHECK(layer->height() == height);
@@ -137,7 +137,7 @@ TEST_CASE("Construct ImageLayer with both mask as part of imagedata and through
137137
.height = height,
138138
};
139139

140-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
140+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
141141

142142
CHECK(layer->width() == width);
143143
CHECK(layer->height() == height);
@@ -175,7 +175,7 @@ TEST_CASE("Construct ImageLayer with invalid channels"
175175
.height = height,
176176
};
177177

178-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
178+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
179179

180180
CHECK(layer->width() == width);
181181
CHECK(layer->height() == height);
@@ -212,7 +212,7 @@ TEST_CASE("Construct ImageLayer with too little channels"
212212
.height = height,
213213
};
214214

215-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
215+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
216216

217217
CHECK(layer->width() == width);
218218
CHECK(layer->height() == height);
@@ -246,7 +246,7 @@ TEST_CASE("Set layer channel with Enum::ChannelID")
246246
.height = height,
247247
};
248248

249-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
249+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
250250

251251
CHECK(layer->width() == width);
252252
CHECK(layer->height() == height);
@@ -283,7 +283,7 @@ TEST_CASE("Set layer channel with int16_t")
283283
.height = height,
284284
};
285285

286-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
286+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
287287

288288
CHECK(layer->width() == width);
289289
CHECK(layer->height() == height);
@@ -319,7 +319,7 @@ TEST_CASE("Set layer channel mask channel with int16_t")
319319
.height = height,
320320
};
321321

322-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
322+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
323323

324324
CHECK(layer->width() == width);
325325
CHECK(layer->height() == height);
@@ -361,7 +361,7 @@ TEST_CASE("Set layer invalid channel"
361361
.height = height,
362362
};
363363

364-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
364+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
365365

366366
CHECK(layer->width() == width);
367367
CHECK(layer->height() == height);
@@ -402,7 +402,7 @@ TEST_CASE("Set layer invalid size channel"
402402
.height = height,
403403
};
404404

405-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
405+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
406406

407407
CHECK(layer->width() == width);
408408
CHECK(layer->height() == height);
@@ -440,7 +440,7 @@ TEST_CASE("Set layer data with Enum::ChannelID")
440440
.height = height,
441441
};
442442

443-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
443+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
444444

445445
CHECK(layer->width() == width);
446446
CHECK(layer->height() == height);
@@ -486,7 +486,7 @@ TEST_CASE("Set layer data with int")
486486
.height = height,
487487
};
488488

489-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
489+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
490490

491491
CHECK(layer->width() == width);
492492
CHECK(layer->height() == height);
@@ -536,7 +536,7 @@ TEST_CASE("Set layer invalid channel"
536536
.height = height,
537537
};
538538

539-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
539+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
540540

541541
CHECK(layer->width() == width);
542542
CHECK(layer->height() == height);
@@ -585,7 +585,7 @@ TEST_CASE("Set layer invalid size channel"
585585
.height = height,
586586
};
587587

588-
auto layer = std::make_shared<ImageLayer<type>>(std::move(data), params);
588+
auto layer = std::make_shared<ImageLayer<type>>(data, params);
589589

590590
CHECK(layer->width() == width);
591591
CHECK(layer->height() == height);

‎PhotoshopTest/src/TestSmartObjects/TestSmartObjectLayer.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ TEST_CASE("Read all supported warps and write image files")
164164
}
165165
else
166166
{
167-
CHECK(result.meanerror < 0.004f);
167+
CHECK(result.meanerror < 0.005f);
168168
}
169169
}
170170
};
@@ -316,8 +316,12 @@ TEST_CASE("Roundtrip layer read-write mixed linkage")
316316
auto file = LayeredFile<bpp_type>(Enum::ColorMode::RGB, 64, 64);
317317

318318
Layer<bpp_type>::Params lr_params{};
319+
lr_params.width = 128;
320+
lr_params.height = 64;
319321
lr_params.name = "SmartObject";
320322
Layer<bpp_type>::Params lr_params2{};
323+
lr_params2.width = 128;
324+
lr_params2.height = 128;
321325
lr_params2.name = "SmartObject2";
322326

323327
auto layer = std::make_shared<SmartObjectLayer<bpp_type>>(file, lr_params, "documents/image_data/ImageStackerImage.jpg", LinkedLayerType::external);

0 commit comments

Comments
 (0)