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

Color picker also shows/detects the color of the pixel grid #13831

Closed
1 task done
Copystrike opened this issue Oct 14, 2021 · 34 comments
Closed
1 task done

Color picker also shows/detects the color of the pixel grid #13831

Copystrike opened this issue Oct 14, 2021 · 34 comments
Assignees
Labels
Issue-Bug Something isn't working Priority-2 Bug that is medium priority Product-Color Picker All things around the Color Picker utility Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@Copystrike
Copy link

Microsoft PowerToys version

0.47.1

Running as admin

  • Yes

Area(s) with issue?

ColorPicker

Steps to reproduce

  1. Open the color picker using the shortcut (For me this is WIN + SHIFT + C).
  2. Open a black image (It does this for every color, but just for this example I go with this color).
  3. Zoom in until you see the white pixel grid.
  4. Select the shadow of the white pixel grid.

chrome_StTYJvIIkn

I know this shouldn't be happening since on my desktop computer this isn't an issue but on my laptop it is.

✔️ Expected Behavior

The color picker should ignore the white pixel grid and pick the color underneath it.

❌ Actual Behavior

The color picker detects the white pixel grid as a part of the color you want to pick.

Other Software

No response

@Copystrike Copystrike added Issue-Bug Something isn't working Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels Oct 14, 2021
@jaimecbernardo
Copy link
Collaborator

I believe this is intended behavior.
If your screen is showing the white grid, that's what's picked by Color Picker.

@Copystrike
Copy link
Author

I believe this is intended behavior. If your screen is showing the white grid, that's what's picked by Color Picker.

Well the thing is the color picker adds this white pixel grid if you zoom in, the original picture doesn't have anything white in it.

This is what I suspect as intended behavior:
4Cic7Xybz6

And this is what happens as of right now:
TfM5aq2Zir

For example in my scenario, I use it to see if there is a hex difference between two colors that may look the same but are not , but since the color picker detects its own grid it makes it really difficult to do that.

@Jay-o-Way
Copy link
Collaborator

@jaimecbernardo can you explain how this is intentional?
Duplicate of #7818, which is closed but apparently not 100% prevented.

@Jay-o-Way
Copy link
Collaborator

@Copystrike can you share some specs about your system? Like cpu/gpu, screen resolution/frequency...

@franky920920 franky920920 added the Product-Color Picker All things around the Color Picker utility label Oct 15, 2021
@Copystrike
Copy link
Author

@Copystrike can you share some specs about your system? Like cpu/gpu, screen resolution/frequency...

Yeah sure @Jay-o-Way!

Here are the laptop details:

System:
Processor: Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz, 1800 Mhz, 4 Core(s), 8 Logical Processor(s)
Installed Physical Memory (RAM) 8,00 GB
Total Physical Memory 7,85 GB
Available Physical Memory 3,69 GB
Total Virtual Memory 9,10 GB
Available Virtual Memory 3,79 GB
Page File Space 1,25 GB

Display:
Name: Intel(R) UHD Graphics 620
Adapter RAM: 1,00 GB (1.073.741.824 bytes)
Bits/Pixel: 32
Resolution: 1920 x 1080 x 60 hertz

It doesn't have a GPU. If you need more information feel free to ask.

Also something I noticed in the first GIF which I recorded in on my computer (which doesn't have the problem) is that it also detected the pixel grid, but I wasn't able to spot it at the time of recording with my bare eyes because it probably updated so fast, but the GIF was able to record it.

@Jay-o-Way
Copy link
Collaborator

Hm, I'm no expert, but i see a 8th gen i5 quadcore at only 1,60GHz. I would say that is the first suspect for a bottleneck. Other then that, no strange specs.

@jaimecbernardo
Copy link
Collaborator

@jaimecbernardo can you explain how this is intentional? Duplicate of #7818, which is closed but apparently not 100% prevented.

This is not intention, indeed.
I didn't understand ColorPicker was adding this grid.
Sorry for that misunderstanding.

@crutkas
Copy link
Member

crutkas commented Dec 29, 2021

We've had this reported but for life of us (#7818), can't repro it which makes it hard to debug. @Copystrike are you running maybe at a higher DPI scale?

@crutkas crutkas added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels Dec 29, 2021
@Jay-o-Way
Copy link
Collaborator

Jay-o-Way commented Dec 29, 2021

@crutkas maybe it's because your computer(s) are just too good, haha. I'm guessing the processor (when there is no actual gpu, or something like that) is the bottleneck.

@crutkas
Copy link
Member

crutkas commented Dec 29, 2021

Jay in theory my laptop should hit this all the time if it was based on spec. There has to be a commonality config or setting.

@crutkas
Copy link
Member

crutkas commented Dec 29, 2021

Maybe we just increase the buffer for adjustment a pixel inward in all directions.

@ghost ghost added the Status-No recent activity no activity in the past 5 days when follow up's are needed label Jan 3, 2022
@ghost
Copy link

ghost commented Jan 3, 2022

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 5 days. It will be closed if no further activity occurs within 5 days of this comment.

@ghost ghost removed the Status-No recent activity no activity in the past 5 days when follow up's are needed label Jan 3, 2022
@Jay-o-Way
Copy link
Collaborator

Jay-o-Way commented Jan 7, 2022

I can not reproduce this anymore. @Copystrike can you? (v0.53 is the latest)
(Even tried to limit the cpu on my all-in-one, from 2011, to 5% but it was all good)

@Copystrike
Copy link
Author

Sorry, I completely forgot about this.

@Jay-o-Way and yes I can still reproduce this issue. (v0.53.1)

@crutkas I have not changed anything to the mouse DPI settings as you can see below in the image.
image

I have no idea how difficult this is to implement plus I am not familiar with C#, but isn't there a way the grid system get its own layer and the cursor is like underneath the grid system layer?

Anyways if you guys have any other questions related to the laptop feel free to ask, I'll try to be more active on this issue and provide as much information as needed.

@ghost ghost added Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams Needs-Team-Response An issue author responded so the team needs to follow up and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jan 7, 2022
@crutkas
Copy link
Member

crutkas commented Jan 7, 2022

It is the fact the grid is on the screen, we grab the exact pixel color. The concept of layers doesn’t exist

@crutkas crutkas removed Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams Needs-Team-Response An issue author responded so the team needs to follow up labels Jan 7, 2022
@crutkas crutkas added this to the Priority work bucket milestone Jan 7, 2022
@Jay-o-Way
Copy link
Collaborator

@crutkas I have not changed anything to the mouse DPI settings as you can see below in the image.

Clint meant the screen/display scaling setting

It is the fact the grid is on the screen, we grab the exact pixel color. The concept of layers doesn’t exist

Nick meant Z-order of (tool)windows

@JulianBrasse
Copy link

I have the same problem, and I think I know why. The grid overlaps with the pixel like so:
CurrentPixelZoomSelector

So as a quite simple solution, as some users have complained that the current selector looks bad anyway (Comment by JolyonJostar) we could either remove the grid and just keep the little square, or use a new one. For example like this:
PixelZoomSelectorExample

The detailed views show that there is no overlap between the selected pixel and the grid:
DetailPixelSelector

It could work, right?
PixelZoomSelector

@Jay-o-Way
Copy link
Collaborator

Looks good @JulianBrasse . Mind if I ask how you made the difference? And did you check different screen scaling settings?

@JulianBrasse
Copy link

To be honest, I couldn't actually implement this (lack of technical knowledge) but I took a screenshot of the earlier method, analysed it and then changed the grid in Paint 3D to a different layout that was still similar to the old one, but (in my opinion) looked slightly better and avoided the overlap of the grid into the pixel.

@JulianBrasse
Copy link

JulianBrasse commented Jan 22, 2022

It is just an idea of what it could be changed to.

@JulianBrasse
Copy link

JulianBrasse commented Mar 16, 2022

@crutkas I can succesfully reproduce the bug on a 1080 x 1920, 60 Hz 6-bit RGB SDR Display. Specifically the HP Envy x360 Convertible 15-cn0xxx. Problem doesn't occur on my big monitor. This is still a problem and I was wondering whether you need more information to continue to fix this.

@crutkas
Copy link
Member

crutkas commented Mar 17, 2022

I know how to produce it. One of the comments I did talks about how.

@Jay-o-Way
Copy link
Collaborator

can succesfully reproduce the bug on a 1080 x 1920, 60 Hz 6-bit RGB SDR Display

And the scaling factor?

@JulianBrasse
Copy link

JulianBrasse commented Mar 19, 2022

image
It is at a scale of 125%

@Jay-o-Way
Copy link
Collaborator

125%

That is consistent with Clint's findings

@KohGeek
Copy link
Contributor

KohGeek commented Aug 12, 2022

Hi, I've dug into this a bit and I realise this also happens at 100% and 200% scaling on a 15" 2880x1800 screen to some extent. The only difference is that it might not be possible to hover on the grid directly as compared to the other non-integer scaling.

Additionally, I am trying to compile the .fx file but I wasn't aware of how, is there any way someone can maybe point me in the right direction? I've identified the GridShader.fx as the fix location but I have no way to compile or debug it properly since it seems to me that the project uses a precompiled cso file and the build process is not integrated.

@ahattangadi
Copy link

I have the same issue here on a Windows 10 laptop. Again, 125% scaling, as Clint and the others here have found.

@IHorvalds
Copy link
Contributor

Hi! I've tried implementing something like @JulianBrasse suggested and played a bit with the size of the main rectangle, but it seems like it's a timing issue more a grid positioning issue.

In the clip below you can see it happening even with a massive main rectangle (sorry for the potato recording quality).

grid-color-timing

However, using the normal grid, modified so it's not drawn inside the sampled pixel (screenshot), seems to resolve landing on the actual grid. The grid colour will at most flash for 10ms before it changes to the correct one.
image

@KohGeek Here's how I compiled the shader file. Not sure it's exactly the corect way, but it works:
Run fxc /T ps_3_0 /Gec /O3 /Fo GridShader.cso .\GridShader.fx inside the Developer Powershell.

IHorvalds added a commit to IHorvalds/PowerToys that referenced this issue Mar 1, 2023
Addresses microsoft#13831. The sampled pixel is now completely avoided by the grid.
Also, this should address the timing issue from the sampling being done
every 10ms, which overlaps strangely with monitor refresh rates at 16.6ms,
13.3ms, etc.
@Jay-o-Way Jay-o-Way added the Status-In progress This issue or work-item is under development label Mar 2, 2023
stefansjfw added a commit that referenced this issue Mar 17, 2023
* Fix ColorPicker sampling colour from the grid

Addresses #13831. The sampled pixel is now completely avoided by the grid.
Also, this should address the timing issue from the sampling being done
every 10ms, which overlaps strangely with monitor refresh rates at 16.6ms,
13.3ms, etc.

* Forgot extra library

* Revert rounding in GetPixelColor

* Ensure EnumDisplaySettingsW returns true

Assigning refreshRate only if EnumDisplaySettingsW returns true,
otherwise defaulting to 60.0Hz.

* Run spellchecker

* Spellcheck again

---------

Co-authored-by: Stefan Markovic <stefan@janeasystems.com>
@stefansjfw stefansjfw added Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed Status-In progress This issue or work-item is under development labels Mar 17, 2023
@stefansjfw
Copy link
Collaborator

Fix for both issues mentioned here (grid drawn inside the sampled pixel and sampling interval) is merged and will be included in the next PowerToys release. Thank you all for the help in discovering what was causing these problems!

@Jay-o-Way Jay-o-Way removed Help Wanted We encourage anyone to jump in on these and submit a PR. Good first issue Good for newcomers. labels Mar 17, 2023
@Copystrike
Copy link
Author

Closing this issue since it's been fixed since v0.69.0 (41997eb) and I can verify that the initial machine where I had this issue does not face this issue anymore. Thanks!

BLM16 pushed a commit to BLM16/PowerToys that referenced this issue Jun 22, 2023
* Fix ColorPicker sampling colour from the grid

Addresses microsoft#13831. The sampled pixel is now completely avoided by the grid.
Also, this should address the timing issue from the sampling being done
every 10ms, which overlaps strangely with monitor refresh rates at 16.6ms,
13.3ms, etc.

* Forgot extra library

* Revert rounding in GetPixelColor

* Ensure EnumDisplaySettingsW returns true

Assigning refreshRate only if EnumDisplaySettingsW returns true,
otherwise defaulting to 60.0Hz.

* Run spellchecker

* Spellcheck again

---------

Co-authored-by: Stefan Markovic <stefan@janeasystems.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug Something isn't working Priority-2 Bug that is medium priority Product-Color Picker All things around the Color Picker utility Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
Status: Done
Development

No branches or pull requests