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.

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

WIP: Bundled profile for reference image #6646

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

Entropy512
Copy link
Contributor

Solves #5575

TBD: Remove the reference image code now that it is superseded?

Also: Currently everything is set to sRGB - we can basically choose any colorspace as long as output = working and the input camera profile is disabled. Any preferences?

Entropy512 and others added 2 commits December 14, 2022 19:44
Also adds a linear sRGB profile to the bundled output profiles - a linear output profile is the
most important component here.

Should eliminate the need for the "reference image" custom function and associated code,
and solve issue Beep6581#5575
Stripped irrelevant keys. Profile to be applied using "Fill" mode.
@Beep6581
Copy link
Owner

TBD: Remove the reference image code now that it is superseded?

Yes, less duplication means less bloat, less code maintenance, less documentation maintenance.

Currently everything is set to sRGB - we can basically choose any colorspace as long as output = working and the input camera profile is disabled. Any preferences?

sRGB seems to mimic the workings of the now-obsolete button, so I'd stick to that.

I proposed a change to the PP3 to strip all irrelevant keys. This makes it clear what the profile is mean to do when you look at it in a text editor, and ensures the profile won't break in RT when a tool is added or modified.

I'm not sure whether you want to include the HLRecovery section which activates highlight reconstruction - it might screw with the colors? Did "Save Reference Image" do that?

@Beep6581 Beep6581 added this to the v5.10 milestone Dec 17, 2022
@Beep6581 Beep6581 marked this pull request as draft December 17, 2022 23:45
@Entropy512
Copy link
Contributor Author

HLRecovery was the default for what I started with - in theory it probably should be disabled, but in practice any image that has been exposed in such a way as for it to matter is one that is fundamentally not suitable to being used as a reference image. I'm fine with nuking it since it's basically a NOP for a proper target shot anyway.

Also fine with stripping the unused/irrelevant keys - I'll review/test that later this week. I'm entering "holiday vacation packing" phase soon. :)

As to sRGB - in general, as long as output = working it should be functionally identical, since "no profile" on input and output=working leads to an effective NOP with respect to gamut transforms.

sRGB linear as an output profile is probably more useful to people for bundling as there are other use cases for it until a better long-term solution for #6644 can be determined and implemented

ProPhoto as working matches the current default working profile of RT (e.g. it isn't a change from the defaults while sRGB is)

Probably usefulness of the bundled profile wins, unless you've got thoughts as far as a more "common" use case as far as gamut for output profile.

@Entropy512
Copy link
Contributor Author

@Beep6581 thinking about this more - I'm not sure what Hugin will do if the input gamut is other than sRGB. If the attached ICC profile indicates linear encoding it does respect that, but who knows what for the gamut.

I do know that if the input is flagged as linear with sRGB gamut, Hugin's output will be linear data that has not had any gamut transformations applied (since we all know it isn't actually sRGB, we just want to ensure that all gamut transforms are no-ops). My use case for the original issue was making it less painful to use Hugin to defish the reference images from a fixed-lens fisheye camera, and it also makes pulling fiduciary coordinates with GIMP a lot easier.

Also, do you know if there's an "ignore user" function in github? There's a block feature but that's probably overkill for the moment.

@Beep6581
Copy link
Owner

Beep6581 commented Jan 4, 2023

HLRecovery was the default for what I started with - in theory it probably should be disabled, but in practice any image that has been exposed in such a way as for it to matter is one that is fundamentally not suitable to being used as a reference image. I'm fine with nuking it since it's basically a NOP for a proper target shot anyway.

@Entropy512 I agree that not performing HLrec is probably for the best considering this processing profile's use case. Thumbs up for merging if you either remove the [HLRecovery] section from the PP3, or set Enabled=false, whichever is more appropriate.

Also, do you know if there's an "ignore user" function in github? There's a block feature but that's probably overkill for the moment.

If you block the user, from your perspective it will be like an ignore.

@Beep6581
Copy link
Owner

Beep6581 commented Mar 10, 2023

@Entropy512 please provide info on how to use this profile before merging to dev. Either directly to RawPedia, or here and then I will add it to RawPedia.

@Entropy512
Copy link
Contributor Author

Apologies for not finishing up/cleaning things on this including some of the other outstanding comments. I'll try to revisit this over the weekend.

I'm finally going to start to have more free time again soon to resume the 2137931270721390 different things I've been procrastinating on lately.

I don't think I have rawpedia access - I have been planning on requesting access, but decided to put it off based on the discussion over on pixls regarding a likely migration to a static site with hugo + pull requests. (that is a separate issue/discussion but I'm willing to help with that if needed, I'm planning on pushing for a similar migration at my dayjob.)

@Entropy512
Copy link
Contributor Author

I've noticed you're back and active again @Beep6581 - any news on the replacement for rawpedia discussed in https://discuss.pixls.us/t/an-update-on-rawpedia/34168 ? It would be preferable to add the documentation in question to whatever replaces Rawpedia, instead of to the old system.

@Beep6581
Copy link
Owner

@Entropy512 nothing in the form of news yet, but certainly much motivation (inversely proportional to available time).

Would be good to have this in 5.10, and it's the kind of thing that does not need to be in beta1.

You could provide just a stub paragraph/list of steps here in plaintext describing how to use it, and I'll add it to the docs.

@Lawrence37 Lawrence37 modified the milestones: v5.10, v5.11 Feb 17, 2024
@Lawrence37 Lawrence37 modified the milestones: v5.11, v6.0 Jul 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants