Skip to content

Fix default orientation and box white balance#268

Merged
antond-weta merged 2 commits into
AcademySoftwareFoundation:mainfrom
remia:fix-orientation-and-wb
May 26, 2026
Merged

Fix default orientation and box white balance#268
antond-weta merged 2 commits into
AcademySoftwareFoundation:mainfrom
remia:fix-orientation-and-wb

Conversation

@remia

@remia remia commented May 4, 2026

Copy link
Copy Markdown
Contributor

Description

Proposing the follow minor changes:

Tests

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable. (Check if there is no
    need to update the documentation, for example if this is a bug fix that
    doesn't change the API.)
  • I have ensured that the change is tested somewhere in the testsuite
    (adding new test cases if necessary).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format before submitting, I definitely will look at the CI
    test that runs clang-format and fix anything that it highlights as being
    nonconforming.

@linux-foundation-easycla

linux-foundation-easycla Bot commented May 4, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

@codecov-commenter

codecov-commenter commented May 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.32%. Comparing base (ef9d3a7) to head (a6efd6c).

Files with missing lines Patch % Lines
src/rawtoaces_util/image_converter.cpp 50.00% 2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (60.00%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #268   +/-   ##
=======================================
  Coverage   91.32%   91.32%           
=======================================
  Files          17       17           
  Lines        3551     3551           
  Branches      527      527           
=======================================
  Hits         3243     3243           
  Misses        308      308           
Files with missing lines Coverage Δ
include/rawtoaces/image_converter.h 100.00% <100.00%> (ø)
src/rawtoaces_util/image_converter.cpp 79.56% <50.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef9d3a7...a6efd6c. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@soswow

soswow commented May 4, 2026

Copy link
Copy Markdown
Contributor

Is there a way we can have a regression tests for changed functionality? Also, this feels like a breaking API move basically - changing default behaviour.

@remia

remia commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

I'm not sure there is an easy to test this but will have a look at the test suite.

Also yes this changes behaviour, considering these are bug fix so hopefully this is for the better (the greybox white balance was non functional and the orientation from multiple conversion I tested was wrong, though if some are relying on incorrect orientation metadata in the source file, the latter change might be an issue but for the general case I'd think it's something we want).

@antond-weta

Copy link
Copy Markdown
Contributor

I'm afraid there is no easy way to test orientations. We don't have any vertical images in the test suite, and ideally we would need multiple. Implementing generation of fake raw test images is on the road map, but until then there is not much we can do.

@remia

remia commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Ok, I'll leave this here for the time being then, trying to sort out the CLA thing.

@remia

remia commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

Should be signed now, checking if the update will pick it up.

@antond-weta

Copy link
Copy Markdown
Contributor

/easycla

@antond-weta antond-weta force-pushed the fix-orientation-and-wb branch from 94850a5 to 6d53e76 Compare May 24, 2026 20:21
@antond-weta

Copy link
Copy Markdown
Contributor

Hi @remia, how is it going with CLA? I'd like to push out v2.1.1 with the recent DNG bug fixes, would be great to have this one included as well.

@remia remia force-pushed the fix-orientation-and-wb branch from 6d53e76 to 5721c7d Compare May 26, 2026 21:04
remia added 2 commits May 26, 2026 22:07
Signed-off-by: Rémi Achard <remiachard@gmail.com>
Signed-off-by: Rémi Achard <remiachard@gmail.com>
@remia remia force-pushed the fix-orientation-and-wb branch from 5721c7d to a6efd6c Compare May 26, 2026 21:09
@remia

remia commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

@antond-weta the CLA was signed last week, unfortunately I don't see any reasons why it fails, "see details" is just a link to the signing page. Wonder if the PR needs to be closed / opened again. Will have a look again tomorrow, sorry about that.

@remia

remia commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

Looks like adding my email to authorised contributors from the company dashboard website was not enough and I also needed to follow the steps in the EasyCLA link, anyway, looks like it's happy now (I tried to update the commits earlier to bump the date in case that was related so had to force push unfortunately).

@antond-weta antond-weta merged commit 6d4276a into AcademySoftwareFoundation:main May 26, 2026
25 checks passed
antond-weta pushed a commit to antond-weta/rawtoaces that referenced this pull request May 26, 2026
…oundation#268)

* Fix flip behaviour

Signed-off-by: Rémi Achard <remiachard@gmail.com>

* Fix wb box

Signed-off-by: Rémi Achard <remiachard@gmail.com>

---------

Signed-off-by: Rémi Achard <remiachard@gmail.com>
(cherry picked from commit 6d4276a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants