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

Add satruation adjustment command. #23

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Bobbsterman
Copy link

@Bobbsterman Bobbsterman commented Oct 10, 2021

Modified MainDlg accordingly and added hotkeys in KeyMap (and corrected a grammar mistake). Needs testing.

Disclaimer: I am not a professional dev, just someone who likes to tinker! I did my best to reverse engineer it but I have no idea if it works: I could not figure out how to compile the project. I got a lot of errors regarding missing files: atlapp.h atlframe.h atlctrls.h ... But I didn't modify any of those so... I'm assuming I didn't setup my environment correctly? Not sure how to though...

…cordingly (and corrected a grammar mistake). Needs testing.
@Bobbsterman
Copy link
Author

Bobbsterman commented Oct 10, 2021

Dunno how to add labels: this is (meant to be) an enhancement

@sylikc sylikc added the enhancement New feature or request label Oct 12, 2021
@sylikc
Copy link
Owner

sylikc commented Oct 12, 2021

I think only I can add the tag. I'll review this against the code when I get a chance. Building is actually pretty easy

Look at https://github.com/sylikc/jpegview/blob/master/readme.txt and you'll see that you need WTL. That's where those missing files are from. I'm using VS 2019 CE for the compiler

…e inversed. Also, tweaked saturation and contrast adjustment increment by 0.01: this brings it in line with the gamma adjustment increment and guarantees the user can always recenter the adjustments using hotkeys alone (since it was an odd valued increment before, this was not always the case).
@Bobbsterman
Copy link
Author

Thanks for the pointer! I got it compiled and tested it. Good thing too: it needed a bug fix because I got my mins and maxs mixed up. I can confirm that works flawlessly now!

I also tweaked the adjustment increment (for contrast and saturation adjustments) to bring them in line with gamma adjustments (0.02 instead of 0.03) and guarantee you can always perfectly recenter them with the hotkeys (assuming you only use the hotkeys – obviously if you click to a random odd number, you will no longer be able recenter with the hotkeys).

@sylikc
Copy link
Owner

sylikc commented Jan 30, 2022

let me test out these hotkey updates

@@ -142,13 +144,15 @@ Ctrl+Shift+Plus IDM_CONTRAST_CORRECTION_INC
Ctrl+Alt+Plus IDM_COLOR_CORRECTION_INC
Ctrl+Plus IDM_CONTRAST_INC
Shift+Plus IDM_GAMMA_INC
Alt+Plus IDM_SHARPEN_INC
Alt+Plus IDM_SATURATION_INC
Copy link
Owner

Choose a reason for hiding this comment

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

so this is a reassignment of the keys from sharpen to saturation right?

I would prefer making saturation (new feature) assign the new key rather than changing the older one

Copy link
Author

Choose a reason for hiding this comment

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

That's right.

Yea, I thought about just using a new shortcut, but thought it would be better to instead keep the shortcuts more consistent? Each one of the 3 modifier keys controls one of the 3 editing sliders on the left of the editing panel. I thought the sharpen slider was really of a different category (since it doesn't actually edit the photo, but instead is just a filter when viewing the image) and therefore should have a distinct type of shortcut. I thought that would be a more intuitive user experience? But I don't have strong feelings about this – I'll let you decide which way to go on this.

@Bobbsterman Bobbsterman changed the title Attempting to add a satruation adjustment command. Add satruation adjustment command. Nov 2, 2022
@sylikc sylikc added the backlog This isn't a priority add/fix at this time label Jan 29, 2023
@Bobbsterman Bobbsterman reopened this Oct 10, 2023
@Bobbsterman
Copy link
Author

Accidentally closed. Mostly wanted to say, I would really appreciate this pull! I like all the improvements you made, but this command is essential to my workflow. I don't care what you set the default hotkey to. Thanks!

@sylikc
Copy link
Owner

sylikc commented Oct 11, 2023

Let me get back to look at this again next week and see what I can do. Thanks

@sylikc sylikc linked an issue Oct 17, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog This isn't a priority add/fix at this time enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hotkey to Desaturate image
2 participants