Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update colorspace retrieval logic #64

Merged
merged 7 commits into from
Feb 5, 2025
Merged

Update colorspace retrieval logic #64

merged 7 commits into from
Feb 5, 2025

Conversation

jakubjezek001
Copy link
Member

@jakubjezek001 jakubjezek001 commented Jan 31, 2025 •
edited
Loading

Changelog Description

  • Enhanced colorspace fetching to prioritize new data structure.
  • Added fallback options for backward compatibility.
  • Improved logging for better debugging of colorspace sources

Additional review information

Loading Image was still in old deprecated way of colorspace maintaining.

Testing notes:

  1. load with clip or image loader
  2. notice that the colorspace set to node is the one defined in representations' colorspaceData

@jakubjezek001
Update colorspace retrieval logic
d86448a 
- Enhanced colorspace fetching to prioritize new data structure.
- Added fallback options for backward compatibility.
- Improved logging for better debugging of colorspace sources.
@jakubjezek001
Refactor colorspace handling in loaders
76a55af 
- Removed redundant colorspace retrieval from load_clip.
- Centralized colorspace setting logic in load_image.
- Added methods to get and set colorspace based on representation and version data.
- Improved readability by simplifying file path handling.
@jakubjezek001 jakubjezek001 self-assigned this Jan 31, 2025
@jakubjezek001 jakubjezek001 added the type: bug Something isn't working label Jan 31, 2025
moonyuet
moonyuet requested changes Feb 3, 2025
View reviewed changes
Copy link
Member

@moonyuet moonyuet left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with updating and switching function with the clip loaded by the old clip loader. Works nicely.
I also tested with loading clip/image function, loading clip works as expected to load the clip with correct colorspace data. But the image loader still gives colorspace error for loading single image frame.
image

@robin-ynput
Copy link
Contributor

robin-ynput commented Feb 3, 2025 •
edited
Loading

But the image loader still gives colorspace error for loading single image frame.

@moonyuet I tried to load a single image frame (clip + image) from single image and I didn't get any issue.
It was from plate and review products. Do you have any error message or any particular context to generate this single image frame ?

@moonyuet
Copy link
Member

moonyuet commented Feb 3, 2025

But the image loader still gives colorspace error for loading single image frame.

@moonyuet I tried to load a single image frame (clip + image) from single image plate and I didn't get any issue. Do you have any error message or any particular context to generate this single image frame ?

No not really. I believe it could be because I am using the old scene to do so.
I re-tested it again with loading image with the texture and also the same clips. they are all working now.
image

jakubjezek001 and others added 5 commits February 5, 2025 16:52
@jakubjezek001 @moonyuet
Update client/ayon_nuke/plugins/load/load_image.py
35f0208 
Co-authored-by: Kayla Man <[email protected]>
@jakubjezek001
Fix variable naming and improve logging
1b1a7d8 
- Renamed `file` to `filepath` for clarity.
- Updated warning log message to reflect new variable name.
- Ensured file path uses consistent formatting by replacing backslashes with forward slashes.
@jakubjezek001
Merge branch 'develop' into bugfix/nuke-loading-colorspace-from-repre…
ec94639 
…sentation
@jakubjezek001
Refactor imports in load_image.py
637d6f7 
- Removed unused imports for cleaner code.
- Kept only the necessary import for loading functionality.
@jakubjezek001
Merge remote-tracking branch 'origin/bugfix/nuke-loading-colorspace-f…
517916f 
…rom-representation' into bugfix/nuke-loading-colorspace-from-representation
@jakubjezek001 jakubjezek001 merged commit 727bf52 into develop Feb 5, 2025
1 check passed
@jakubjezek001 jakubjezek001 deleted the bugfix/nuke-loading-colorspace-from-representation branch February 5, 2025 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moonyuet moonyuet moonyuet approved these changes

@iLLiCiTiT iLLiCiTiT Awaiting requested review from iLLiCiTiT

@robin-ynput robin-ynput Awaiting requested review from robin-ynput

@MilaKudr MilaKudr Awaiting requested review from MilaKudr

Assignees

@jakubjezek001 jakubjezek001

Labels
type: bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jakubjezek001 @robin-ynput @moonyuet