Add simple color correction #7806
Conversation
|
Hi, I am curious how this gets around her images being washed out. From the numerous Direct* calls, I am guessing you're now using Direct* and some type of shaders to do color correction that is closely tied to Direct*. Is there any alternative methods you've investigated for basic color correction? |
|
@BrycensRanch is it washed out with this version? It won't be perfect overall, but should look quite ok when taken a screenshot from some window application like edge opened to https://www.wide-gamut.com/test/image-hdr - check how the screenshot will look on this page. With the fix should be similar to this:And without the fix:Also proper HDR screenshot made with separate app - not color mapped, just raw HDR outputIf it does not look ok it might be some wrong configuration, you can check this by checking your display options, right click desktop, display settings, advanced settings and tell me what peak brightness you see there, and also what screen do you own? Like this |
|
Jaex has shown no interest in providing HDR support because they don't personally have a HDR monitor. That's baffling for an open-source project with hundreds of community contributions for the key-holder to then decide that they don't want to consider the same community that put them where they are, but hey, that's how git works. This has been tried and raised for years and has gone nowhere. You're really better off making your fork a true fork, since there's little to no chance that there will ever be any acknowledgement or support officially. Sincerely, |
|
@Jaex please merge this PR, this fixes the most popular issue :) |
Hi, I should've clarified. I've looked at the contents of this PR. However, it's missing documentation on how to maintain it. The person who originally wrote the code is off writing drivers somewhere else. While this PR is a few thousands lines of code, it's complexity cannot be understated. Especially when ShareX primarily uses GDI+ instead of DirectX related apis so you're throwing Jaex into something the program doesn't use. The preview on HDR monitors is still borked. In terms of technicals, The PR doesn't seem to work right on my HDR10 monitor (LG ULTRAGEAR 32GN50T-B) Also, the dependency it uses, SharpDX is unmaintained. I am using Google Chrome 134.0.6998.89 In terms of the laysman, the memory usage after taking a screenshot (running the DirectX code) is magnitudes higher than normal. When taking a screenshot, it spikes up to 300MB instead of the 100-250MB now. (All of the memory measures were done in Compared to Snipping Tool which only uses 200MB when taking a screenshot and its preview isn't borked. Application sizeShareX (with installers/uninstallers & ffmpeg removed)ShareX-HDROf course, the overblown images are taken with WinRT off. It seems like it slightly improves the overblown images to be useful however I still think the result is not acceptable, especially for a program like ShareX. You may compare it to the overblown images and say that's better, but taking HDR screenshots is unsupported meaning it doesn't effect the bottom line of features that are properly implemented. No WinRT (regular, unsupported behavior)WinRT (drastically better in some areas, but worse in others, not something you can ignore, even an amateur can see it)Snipping Tool on Windows 11 24H2 (with HDR screenshot color corrector on)My SetupBrycen@VENATOR-PH315
--------------------
OS: Windows 11 Pro x86_64
Host: Predator PH315-54 (V1.15)
Kernel: WIN32_NT 10.0.26100.3194 (24H2)
Uptime: 3 days, 12 hours, 44 mins
Packages: 425 (pacman), 4 (scoop), 15 (choco)
Shell: bash 5.2.37
Display (LG ULTRAGEAR): 1920x1080 @ 165 Hz in 32" [External, HDR] *
Display (E24): 1920x1080 @ 75 Hz in 24" [External]
Display (BOE08B3): 1920x1080 @ 144 Hz in 16" [Built-in, HDR]
DE: Fluent
WM: Desktop Window Manager 10.0.26100.3037
WM Theme: Custom - Blue (System: Dark, Apps: Dark)
Font: Segoe UI (12pt) [Caption / Menu / Message / Status]
Cursor: Windows Default (32px)
Terminal: Windows Terminal
CPU: 11th Gen Intel(R) Core(TM) i7-11800H (16) @ 2.30 GHz
GPU 1: Intel(R) UHD Graphics (128.00 MiB) [Integrated]
GPU 2: NVIDIA GeForce RTX 3060 Laptop GPU (5.86 GiB) [Discrete]
Memory: 51.46 GiB / 63.78 GiB (81%)
Swap: 6.65 GiB / 30.01 GiB (22%)
Disk (C:\): 458.14 GiB / 473.96 GiB (97%) - NTFS
Local IP (Ethernet): 10.0.0.140/24
Battery: 94% [AC Connected]
Locale: en-USConclusionAs is, I don't see why this PR would be accepted. The behavior is still incorrect. |
|
@BrycensRanch Higher memory usage is expected, not only currently the app still need to support 2 flows, but also its getting much more raw data so just more data per pixel is needed.
Ofc it's not using GDI because thats legacy API that does not support anything needed, the whole issue here is that windows does not provide good api for this. Note that windows snipping tools result is also incorrect, and thats just sad considering its directly from microsoft. For now i just want to provide people with some solution, until either MS wakes up and gives us any good api, or someone can provide better color correction approach. If maintainers of this repository want me to improve something to merge it... im here, but so far its rather clear they just don't want to hear about HDR no matter what so I don't want to invest time into smaller issues (like using older library) if it won't affect chances of this PR going anywhere sadly... I can try to improve it if you actually will provide the needed data for me to try to debug it. |
Debug Information you wantedThe HDR-calibrated profiles were added after my response to see how they'd affect the MR despite your recommendation to do so. Why? Because when users have HDR on, they might even calibrate their profile to their screen. Here's what I got afterward: This was the result after the profiles. While you mention that the Snipping Tool is also incorrect, it's what users would hope it to be. The HDR calibration seems to have borked Snipping Tool too. I’ve been using ShareX for a few years now, and it has always been consistent and reliable, thanks to Jaex’s efforts. I don’t see a problem with the maintainer wanting to maintain the quality that users expect from ShareX. Even if this merge request were approved today, it still wouldn’t match the capabilities of other screenshot applications with good HDR support. You didn’t address how you would improve color accuracy, which is the core of my feedback. The APIs needed to support HDR just don’t seem to be adequate. While true that the application is processing more raw data, the memory usage on just 3 1080p monitors taking a screenshot is uncharismatic of ShareX. To stress my point about it, I can already smell a new issue about ShareX running for a while taking some screenshots, and eventually, you'd see ShareX at 740MB+ of memory idling & just staying there. How did I get to this result? Just simply spamming the print screen button with no screenshot delay for 20 seconds, but you get my point. :p On the latest release of ShareX, such screenshot spam doesn't get nearly that high. It gets to 323MB but QUICKLY climbs down to 130MB (You can tell this is the MR by the GPU usage since ShareX doesn't use the GPU) About the maintainer not wanting to "hear about HDR support" since they don't have an HDR monitor, this means the main person handling issues is unable to fix any issues that may arise & while your word to help maintain this is great, you're not imbushuo. This MR does not have minor issues, it has big gaping holes in its implementation. Scrolling capture, Video capture, etc not being implemented. This means it will be an inconsistent experience for even a novice who will still turn HDR off once they're told by the volunteer support team that it simply won't work as they'd hoped. This is what currently happens regardless. |
No one will put their time into this if maintainers of this project don't care. As it would be just wasted time. It all can be done, but its not worth the time if there is no will for any cooperation. And yea, your set peak brightness seems to be off from any spec and review for your screen that points to 360-370 and not 295. The color correction used in this code depends on this setting to provide best results, if I change mine to something invalid it will also start looking bad.
No? its what you like maybe, maybe for your screen it works nicer. No idea what snipping tools do, I think they just assume some constant brightness, so people with better HDR screens often end up with screenshots that are still complete unusable garbage. Thats the reason I even resurrected these changes, as snipping tools are just unusable with my screen, everything looks washed out and unrecognizable. So at most, both tools are bad, but this PR at least takes your configuration into account, just can't do much when its invalid. EDIT: there is one other thing I hope to try out soon, maybe over the weekend, that maybe would work without the correct peak brightness setting, but due to low interest in this PR and that it works good enough for my needs... not much motivation to pour additional days into that. |
This is so disingenious. There has been no guidance, or input from any of the project's maintainers. there has been no guidelines set on what WOULD make a PR like this meet ShareX' lofty standards. So far, the maintainers' response to the growing problem of HDR support has been to pretend the problem doesn't exist, and to provide no support or dialogue to people who DO try and work things out. and then, amazingly, by ignoring PRs like this, they get marked as stale and the problem goes away! A million and one excuses can be created to shut down something like this, including the utter nonsense of "memory usage". Each pixel can use 2-8 more bits, yes, that would multiply the memory usage of the application, that should intuitively compute to anyone that understands how pixel depth works. What do you want? a new novel memory compression algorithm to go along with the PR? A solution that perfectly fits ShareX would only be possible if core members of the ShareX team actually bother get involved with developers that are willing to put in the work to make it work. |
|
Just my two cents - it might be time for a hard fork of ShareX. I was sympathetic when the issue was regarding maintainers not having a HDR monitor to target or test implementation. However, at this point the community has put in a large degree of effort into finding a solution to this pretty considerable issue and is getting either ignored or actively blocked in regard to merging or implementing the solutions which have existed for a while. Both my monitors are currently HDR and i have no way of taking screenshots without washed out colors. This has become a blocker for use of ShareX for me, and to an unreasonable degree, considering the solutions people have attempted to provide. This is hostile behavior towards users, and I think enough due diligence has been made attempting to resolve these concerns that a fork as a response is justifiable. |
|
@polyjitter well my fork is usable, tho currently it will still prompt you to install updates for the original version... I guess I could swap that...and maybe then actually look deeper into improving the correction as it won't be pointless anymore if someone will be using it I guess |
I do think you have a good start here :) The problem is you're not really getting any guidance from project maintainers on what needs to be fixed. The message a bit above does give a good explanation on what still needs to be fixed - video shots, scrolling shots, etc. I think there's enough people affected to want to use a maintained hard fork. For anyone watching this MR - does anyone want to get a working group going with some other HDR users, to talk out a fork and make sure there are some basic governance and maintenance plans? On Matrix or something :) |
|
@shimmer-n-shine @polyjitter you folks' entitlement is off the charts. the project is licensed such that you may fork and change it as you will. at least for @shimmer-n-shine, instead of learning the skills to change the project to their liking or using someone elses fork, or finding an alternative, they've decided to complain for (by their own admission) three years about it. you seem to not wholly understand that @Jaex owes none of us absolutely anything at all; not even a reply, or recognition, or the time of day. they put out a very handy and very feature-rich software and very generously offered it up as open source and licensed it appropriately, and they can do whatever they want with it. if you want a feature added, get off your entitled high horse, fork the repo, add the feature, and get on with your life. the maintainer has made it clear by inaction that they are not interested in the feature, and that is their right. did i come here looking for hdr support too? yes, i'll try the fork. if that doesn't work i'll find some other alternative and get on with my life instead of pretending i am owed from someone that has not promised anything to me. i hope you find happiness PS for people that don't care for the feature-set but just want screenshots in hdr to work - snipping tool in windows has a setting in the app to correct for it. |
If you had done the bare minimum to check, you'd see this is my first interaction on the ShareX repo. While you're trying to derail the pull request, you might want to actually read what was written. Open source software isn't just "make a fork lol". If everyone just made their own forks, no one would ever need to PR to any public repository, so there's clearly something you're missing. Every commit has a burden of maintenance, every fork needs to be kept up to date with upstream while avoiding breaking changes. The second you make a fork, you either commit to that version, or you put in extra work to keep it up to date. Everyone benefits if changes get merged upstream, instead of fracturing a project into innumerable forks, that's quite literally why pull requests are part of open source etiquette. The work has already been done by other people, even before this latest PR, I don't know why you seem to think that it would somehow give me more credibility if I redid the work others have already done? Maybe instead of running headfirst without reading and making assumptions, you could actually engage with other people that have commented in this thread. |
|
Are there any updates on this getting merged? HDR support has been unnecessarily ignored and bikeshedded - the original issue has over 150 comments, this is one step towards completely fixing the issue and ending this drag of a problem. Can confirm that this PR works and completely fixes the issues with HDR. |
|
Yes, I have to turn off HDR to take "normal" screenshots. Please merge! |
|
instead of comments like that it would be better if people would test the HDR version and see if it does produce good results for them https://github.com/GotoFinal/ShareX-HDR/releases |
|
@BrycensRanch Can you stop nitpicking here? We don’t need scrollable screenshots or video capture, and we have enough memory to capture in HDR. We just want a way to capture HDR screenshots. |
|
build away the dependency to SharpDX and gogogo! :) ie implement the same things which were used in sharpdx into this fork. |
Thank you for bringing to that my attention. I have now done so many tests for the issue I had when HDR was enabled; overall, this build has No Difference in my case when testing our website , it is only one I have issues with the BTW, even with the built-in Windows screenshot app. So, I decided to test other open source screen apps to understand their difference but no success, so, then I tested some proprietary ones and small success. Here are the results: Tested OSS ([issue trigger]):
Tested Proprietary ([issue trigger]):
Thank you for your efforts, after spending a ton of time testing various solutions, it seems to be a complicated issue. So, for my use case I will just stick with web-capture for websites like that, I would not worry about the support, although still needed, it is complex issue which may need to await Windows/Dotnet native support. |
|
@BirukTes Did you enable the WinRT setting as shown on release page? I can't also see the model of your display but i doubt its 10 000 nits, so seems like it also have some weird invalid calibration... i guess having completely invalid windows calibration is somehow much more common than I would expect... |
Wow, THANK YOU! Now, no other app comes close to this build of ShareX from the list I shared previously, we can use the region, record and all the existing features working greatly. However, I think the setting should have been enabled by default, at least for the debug build. Edit, just tested few features:
|
|
Tests I would love to see this PR merged. It's not perfect, but I don't think it can be without direct support from Microsoft. The best version of HDR screenshots right now, as others have pointed out, seems to be GameBar, which outputs to a new and poorly-supported format. There might also be variations in quality based on HDR calibration. P.S. If you're using vanilla Windows HDR and you have Windows 11 version 24H2 I highly recommend giving the calibration tool a try; It has been tremendously improved. |
|
|
|
Calibrate your HDR |
already did |
So, to give some feedback for a fringe use case @GotoFinal , I run a 1000 nit capable monitor that I don't actually use as a 1000 nit monitor. More often than not, for games (which are the only type of media I'd typically be taking screenshots of in my case), I typically use the windows autohdr function. It's not perfect, but it's great for adding a BIT of extra depth without sacrificing my ability to stream to friends on various platforms because this isn't the only common tool that can't handle hdr yet. (Lots of games also have awful HDR implementations, in particular with handling UI, so I also tend to use autohdr for that reason as well). Checking the code, I can see that you seem to be doing some super clever stuff where you check for the specs of the monitor and base the amount of correction on that! But unfortunately, what this means is that when I use your fork, all monitors look great in screenshots EXCEPT my only HDR monitor, as that's the one I use autohdr for, and even when I use native HDR in games, I still set it to MUCH lower peak values than my monitor can support, meaning that all of my screenshots come out extremely dark compared to what your examples show post correction. My suggestion would be to include a setting where we can override the values your fork looks up with our own. Or at least a slider that adjusts how "strong" the correction is. This way, I could at least find a good "middle ground" setting that's good enough for what I need it to do, without you having to change how you handle finding a monitor's "Default" configuration! Really, I'd love to see sharex actually implement captures at whatever the native bit depth is for a given monitor, but this workaround seems to work great for the most users, and I get that that sharex isn't likely to implement this feature anytime soon, so I wanted to give this feedback at the very least! |
|
@Festive4020 can you check the new version? Ive changed the whole approach, there is few settings to adjust too, but hopefully they are not needed anyways. It should now do a better job to just adjust to whatever content is on the screen. https://github.com/GotoFinal/ShareX-HDR/releases/tag/v17.1.0-hdr Also @OnyxL |
|
I am currently testing on ShareX 17.1.0 Portable. Startup path: 2025-06-20 13:27:43.615 - OpenFolder(C:\Users\Brycen\Downloads\ShareX-17.1.0-portable\ShareX\Screenshots\) failed.:
System.ComponentModel.Win32Exception (5): An error occurred trying to start process 'C:\Users\Brycen\Downloads\ShareX-17.1.0-portable\ShareX\Screenshots\' with working directory 'C:\Users\Brycen\Downloads\ShareX-17.1.0-portable'. Access is denied.
at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
at ShareX.HelpersLib.FileHelpers.OpenFolder(String folderPath, Boolean allowMessageBox)Region capture/Scrolling capture is broken when having monitors plugged into a different GPU. Having ShareX open files/folders doesn't work. Monitor capture & window capture work fine. Fullscreen capture isn't. System.ApplicationException: HDR screenshot failed
---> System.Exception: 💀 We currently don't support screenshots across multiple GPUs
at ShareX.ScreenCaptureLib.AdvancedGraphics.Direct3D.ModernCapture.CaptureAndProcess(HdrSettings hdrSettings, ModernCaptureItemDescription item)
--- End of inner exception stack trace ---
at ShareX.ScreenCaptureLib.AdvancedGraphics.Direct3D.ModernCapture.CaptureAndProcess(HdrSettings hdrSettings, ModernCaptureItemDescription item)
at ShareX.ScreenCaptureLib.Screenshot.CaptureRectangleDirect3D11(IntPtr handle, Rectangle rect, Boolean captureCursor)
at ShareX.ScreenCaptureLib.Screenshot.CaptureRectangleNative(IntPtr handle, Rectangle rect, Boolean captureCursor)
at ShareX.ScreenCaptureLib.Screenshot.CaptureRectangleNative(Rectangle rect, Boolean captureCursor)
at ShareX.ScreenCaptureLib.Screenshot.CaptureRectangle(Rectangle rect)
at ShareX.ScreenCaptureLib.Screenshot.CaptureFullscreen()
at ShareX.CaptureFullscreen.Execute(TaskSettings taskSettings)
at ShareX.CaptureBase.CaptureInternal(TaskSettings taskSettings, Boolean autoHideForm)I closed my laptop lid (connected to my secondary integrated GPU) And here is the screenshot I got I understand the API you're using for capture doesn't support multiple GPUs. Maybe you could use GDI, assuming the other monitor isn't HDR itself? Screen recording my monitor, I got something gray covering the screen??? It doesn't matter if the monitor is HDR or not. Untitled.video.-.Made.with.Clipchamp.mp4Ignore the Imgur error, it is because Imgur doesn't like my IP address without credentials. Non-HDR content on a HDR-enabled monitor looks good too.
Thank you for all of your hard work on this PR. Please note, I did not change the settings. I did recalibrate my monitor, though! |
|
@BrycensRanch Screen recording will not work with HDR, but should work the same as before, so need to fix that too |
This is just manually rebased old PR #5565 by @imbushuo
Made it as new PR as they don't seems active, but I'm totally fine this being added back to original PR and re-opened.
I've only added small change to automatically fetch the HDR parameters of the screen for a bit better results.
I don't know much about color correction myself, the code provided by imbushuo seems to be doing good enough work, arguably better than default windows tool - as long as your screen is not incorrectly calibrated manually to a different (invalid) values than the actual physical screen.
It does not affect anyone without HDR screen enabled, works fine if not all screens are HDR too. Just to be sure it needs to be enabled manually/can be disabled:
Sadly it might not work in full screen applications like games, as they often break HDR as soon as they are interrupted.
There are few possible options to do this differently, as @Kaldaien suggested #6688 (comment)
SKIV can be used to capture the desktop with full HDR support, but that would require more changes and getting it to work as library, and is still not a full solution due to Special K needing to inject extra DLLs into some games to make HDR capture work (broken with current solution too) - and that breaks/triggers anticheats. Other option there would be to use OBS DLL for that, as that one is usually allowed to be loaded in, but no idea how to actually use it.
Note: any mention of stuff "breaking" just means that the image will look wrong, as bad as it is without these changes but not worse than it is currently.