Just typed this as a reply in the old report but you were faster :D


First of all, thanks, Gil for fixing the bug as fast as you did and sorry that I was not able to participate for some time now, this semester brought more work with it than I initially thought :/

Regarding the additional pixels around the window borders: I also noticed those with my first simple patch. But while playing around for a bit, I noticed that they indeed seem to be a part of the window. You can check this by bringing your mouse cursor close to the window border. You'll notice that for the bottom, left and right window sides, your cursor switches to the resize-window-cursor before you even reach the real, visible window boundaries. My guess is that this is the area where the window border would have been if Windows 10's windows still had borders on all four sides. 

If this is correct, you can't really cut this part away because on older Windows versions it might actually contain pixels that belong to the window. I will test this on a Windows 7 system in my university tomorrow and report back.

Oh ok. If that's really the case, it's a bit crappy that Windows don't have proper accurate API.

Anyway tell us how the tests go. :-)

I am very happy to help and contribute from my experience.

According to my experience with Windows, I confirm that Microsoft makes life difficult.

But I believe there is no such thing as "no solution" in most cases.

I know about the problem. It's a bug that always existed also before my patch.

This is one of the few bugs I could not solve.
I did found a kind of start of a solution .. but it creates problems in other cases. I need to check it further.

Is this maybe some theme-related shadow?

Created attachment 372109 [details] [review]
A possible solution but not tested

This is a possible solution. it may be a start.

I changed the line 
if (!GetWindowRect (selectedHwnd, &rect))

to
if (!GetClientRect(selectedHwnd,&rect) || !ClientToScreen(selectedHwnd,&rect))


It's important to note that I was not able to check it out properly -
I have a problem in build system now so I could not build gimp.

However, it should work. I am surprised.. I did a test code and always it gave me the correct and accurate RECT data.

I have not found a case where I get wrong RECT data.
So maybe it's a solution. But it needs loads of testing!

Sorry,
I was wrong. It should be this:

.............
.............

static gboolean
doCapture (HWND selectedHwnd)
{
  RECT rect;
  POINT point;

  /* Try and get everything out of the way before the
   * capture.
   */
  Sleep (500 + winsnapvals.delay * 1000);

  /* Are we capturing a window or the whole screen */
  if (selectedHwnd)
    {
      ZeroMemory(&point, sizeof(POINT));
      if (!GetClientRect(selectedHwnd,&rect) || !ClientToScreen(selectedHwnd,&point))
      /* For some reason it works only if we return here TRUE. strange... */
          return TRUE;

      rect.left = point.x;
      rect.top = point.y;
      rect.right += point.x;
      rect.bottom += point.y;



.............
.............

As I feared,
It does not work properly for everything.
If it build and you try it, it may give good result on lot of windows..
but for notepad for example it will not capture everything. the title border of the notepad window will not be included in the image.

I think we can still use combination of:
*Output of ClientToScreen
*Output of GetClientRect
*Output of GetWindowRect

This is probably a crazy trick .. but it can solve the problem and that is what matters.

Tomorrow I will submit a patch based on the idea. Assuming it will work correctly.

Created attachment 372115 [details]
Screenshot, taken on Windows 7

As promised I just checked on Windows 7 and attached the result. As you can see, there was a border around the window that is pretty much the same size as the small border of "wrong pixels" you see on Windows 10.

Regarding Michael's idea that it might be the result of the window shadows:
This was my first thought, too. But Windows 10's windows have shadows all around them and at least for me the wrong pixels only occur on three borders: left, bottom and right. For the top border everything is fine although there is a shadow. 

Now we have the following problem: We can't simply check for the Windows version and cut a piece of the image away if we are on Windows 10 because someone might install a different theme that brings back the border around the windows. 

@Gil: What is your solution supposed to do? 
I just tried to understand what you are doing and it does not make much sense to me right now. If GetClientRect succeeds, the left part of the condition is 0 and ClientToScreen gets executed - which overwrites the result we just got from GetClientRect. If GetClientRect does not succeed though, the left side is 1 and ClientToScreen is short-circuited.

@Simon Müller,
Look at the code in comment 7.

If one of the functions fails in the condition then we return TRUE in order to avoid continue.

The only way that the condition will not return TRUE is if both functions in the condition returned non-zero.

The condition first check if the first function failed.
If in the left side of the condition we did not got  false, then it must be true.
otherwise, in case that we got false (zero), the condition will immediately return to stop the function.

In the second case that the left side is not false (non-zero), then it must be true. (so the first function call did not failed) so the machine will continue to the next part of the condition - "is the second function failed?"
If second function returned zero=failed=false then the answer to the question is yes -> the condition will return to stop the doCapture function
if no, (that means that the second function returned non-zero = not failed) then condition will not return to stop the doCapture function.

in such a case, both functions did NOTNOT succeeded = succeeded we should continue to run the doCapture function.

This can also be easily explained by boolean algebra
http://prntscr.com/jij8hy


Maybe I'm wrong .. I'd love to learn from mistakes.
In any case, with regard to the solution itself, I did not behave professionally.
Next time I will check the patch in depth (as I did before) before I publish.

Oh, my brain somehow projected the snippet from comment 5 into the one in comment 6. This is why I thought you were overwriting &rect with the call to ClientToScreen. Sorry for that...

> In any case, with regard to the solution itself, I did not behave professionally.
> Next time I will check the patch in depth (as I did before) before I publish.

No need to self-blame yourself. You are doing a good job and sharing ideas on bugzilla (even when still untested) is actually a very good practice.

And if you make mistakes, so what? Professionals do make mistakes too. This is not a problem at all and nobody will chastise you for it. :-)

Thanks Jehan!


Simon Müller,
I now understand your findings

You seem to have come to the correct conclusion that the borders are simply invisible. in Windows 7 we do not feel there is a problem at all because the same borders are not invisible.

As you say, we may cut the borders but then could be a bug when the user changed the theme...

Therefore, I believe that a perfect solution to the problem is not possible.

Perhaps we should think of a solution based on image processing.
It can sound crazy and it's indeed crazy.

This is not the first time doing crazy ideas. I have already written an algorithm that separates between pixels that are texts and pixels that are background.. 

This time I have an idea for algorithm that find sharp corners. it's starting point is the output of GetWindowRect.

Jehan, (Or anyone else that in charge) Do you want me to work in that direction?

> Jehan, (Or anyone else that in charge) Do you want me to work in that direction?

I want you to work in any direction you feel appropriate. :-)

If you both believe that there is no correct solution to the problem because of invisible borders, then do not hesitate to close yourself the report as NOTABUG.

If you think there is a solution, but it is not worth the effort (because too complicated/convoluted), then nobody will resent you for not doing it (then you can close as WONTFIX).

Basically: contribute in a comfortable way. This is how we hope you will stay around, and hopefully contribute more and more! ;-)

But really *seriously*: I want you to choose if you think this is worth the effort. I personally feel that your idea is a complicated path and sounds like over-engineering. If you really want to try and it turns out to work without any drawback, why not. But don't do it if you think you'd be wasting your time for a very minor issue in the end. I'm perfectly fine with just closing this report as a WONTFIX.

By the way, regarding "who is in charge", as far as we are concerned, for all Windows-specific matters, you could both be considered in charge if you continue contributing on regular basis. You are what we are the closest to Windows expert since we are seriously lacking Windows developers. We have a loooot of other bugs which can be fixed on the Windows build and we welcome any Windows developer to help us. So don't worry, if we close this one now, you can start right away on another bug. :p

Fun fact: Microsoft seems to have had this same exact problem: https://blogs.msdn.microsoft.com/oldnewthing/20171024-00/?p=97275

But they somehow fixed it for their screenshot tools (Alt+PrtScr, Snipping Tool).

After all, there has to be a better way to do this without falling back to some hacky analysis of image contents.

Aaand right after posting I found this: https://www.cyotek.com/blog/getting-a-window-rectangle-without-the-drop-shadow ...

This article mentions a function called DwmGetWindowAttribute (from dwmapi.dll, available on Windows Vista and higher, docs: https://msdn.microsoft.com/de-de/library/windows/desktop/aa969515(v=vs.85).aspx) that is supposed to be able to return the correct window position without that additional border.

The example uses C# but it should be rather easy to transfer it over to C.

(In reply to Simon Müller from comment #15)
> Aaand right after posting I found this:
> https://www.cyotek.com/blog/getting-a-window-rectangle-without-the-drop-
> shadow ...
> 
> This article mentions a function called DwmGetWindowAttribute (from
> dwmapi.dll, available on Windows Vista and higher, docs:
> https://msdn.microsoft.com/de-de/library/windows/desktop/aa969515(v=vs.85).
> aspx) that is supposed to be able to return the correct window position
> without that additional border.
> 
> The example uses C# but it should be rather easy to transfer it over to C.

Sounds like a better option!

Jehan, I'm not sure I would have done it anyway. It is indeed too much work for a small bug.

Simon Müller, well done.
You did what I did in the previous bug - giving a working solution to a problem previously considered as unsolvable. 

Good work!

Same as Comment 15.

'Keyword' = 'DwmGetWindowAttribute() API' or 'DwmExtendFrameIntoClientArea() API'
https://msdn.microsoft.com/en-us/library/windows/desktop/aa969512(v=vs.85).aspx
Ref; ja/jp) http://www.webtech.co.jp/blog/os/win10/8445/  (Japanese! But Pictures are useful)

PS: Please Not Add Me into CC list.  :)

I got time, so I wrote the code, Please see it. It is VB.net.
For me, there are almost no cases to operate the API, so please take a look at that.
Although the title bar and the part of the window frame are unfine, the client area seems to have no problem.
I have not had time anymore, so it's end.
Separately, how about overlapping the acquired title part etc?
Also, because it is GDI, I feel that the image becomes a bit dirty.
Is it helpful? :)

Created attachment 372142 [details]
VB.net Code (Sample)

for Comment 19

Created attachment 372143 [details]
Vb.net Executing Screen Shots

Thanks ShiroYuki. Unfortunately we cannot use VB.net code in GIMP. But maybe it will help others to write a patch by converting to C calls.

(In reply to ShiroYuki_Mot from comment #18)
> PS: Please Not Add Me into CC list.  :)

Removing ShiroYuki from Cc as per-request.
I believe you can do it yourself, by the way, just so you know! ;-)

Dear Jehan, Thanks for your removing.  (Comment 23)

To me, Windows APIs are too high hurdle...
Perhaps, I'll never be able to resolve the problem of a title-bar popping out of the screen area and the quality of capturing image. Never...
Because of I have too little of that knowledge. X(

... There are a way to ask questions in the msdn forum category of your country (ex.; C++) to get the key to solution.

I hope GIMP's Great Developers resolve it. And I am waiting. :)

Hello, I see that the person who suggested the excellent solution has not yet released a patch. 

Are you waiting for someone else to implement the patch?
I can try to implement the idea you suggested (using the dwm api). I just do not know if you're working on it. If not then I can start working on it now :) !

Dear Gil Eliyahu, Is this comment (Comment 25) for me?.

If Yes, the answer is that 'Please Use my idea!'.
(Is my idea helpful?)
I can not write VC++, GCC and C. (only vb.net).
Therefore, a patch ... Oh, Nothing at all...
My position is an end user who can write a script a bit. (... not a developer)
(And really I don't understand English ... Sorry, My Poor English...)

Before that, First!, I'd like to say you that the great function you proposed is that everybodies smile. :)
and, we (the end users) get a new great Developer!. Thanks a lot. :) :)

PS; Removing me from CC list after posting this comment.

Hey Gil, yeah go ahead - I tried to do it myself but I somehow screwed up the dependencies - so now I can't compile GIMP (and therefore don't test the stuff I write) until I sorted everything out. 

I compile with msys2 which has the forbidden new version of glib in its default packages. But as soon as I compile an older glib myself, GIMP's configure script suddenly complains about missing GTK+ and glib-networking.

Ok, I'll try to implement the patch :)

ShiroYuki_Mot, I'm glad to hear your gratitude. Thank you :)

Created attachment 372223 [details] [review]
Fix bug 796151 - using dwm api

Hello, I'm done developing the patch.

Please check it out. It need testing undew Windows 7 , vista.
I tested it on Windows 10.

Make sure I did not did anything wrong.


And I think you need to also mention the name of Simon Müller as the a person who gave the idea how to fix the bug



I must say that I also had a problem with bulding gimp.
so I did:
"git reset --hard 1139c00721da11cd1fae1d78b16a02c404346d6b"

it is commit:
Author: Ell <ell_se@yahoo.com>
Date:   Sat May 12 09:23:40 2018 -0400

    app: in GimpHistogramEditor, avoid calculating histogram of detached layers
    
    ... which results in CRITICALs.  This can evidentially happen during
    gimp_image_remove_layer() under certain circumstances (see comment
    in the patch.)


That's the only way I could work on it and check it out.
Please fix this build problem that happens after this commit.


I hope I helped!

Review of attachment 372223 [details] [review]:

::: plug-ins/screenshot/dwm-api-win32.h
@@ +2,3 @@
+ * Copyright (C) 1995 Spencer Kimball and Peter Mattis
+ *
+ * Magnification-win32.h

It should be:
dwm-api-win32.h

@@ +48,3 @@
+void UnloadRequiredDwmFunctions(void)
+{
+	if (dwmApi) return;

It should be:
if (!dwmApi) return;

Review of attachment 372223 [details] [review]:

Thanks for the patch Gil.

Why do you redefine DWMWINDOWATTRIBUTE? I can see it is in a public API of Windows. Why don't you just include necessary headers?
It's not a very good idea to copy code this way.

Also do you need to create another file just to load/unload an API?

Finally be careful, you add again a lot of tabs, trailing spaces, and various coding style issues (misaligned braces, etc.).

It just did not work when I included
#include <Dwmapi.h>
It did not worked.

DWMWINDOWATTRIBUTE is defined under Dwmapi.h

The problem is that according to Microsoft docs I need also to include Dwmapi.lib (the same case as with magnification api)
There is no way to include lib files in your build system.

So I did custom dwn api header that that eliminates the need of lib file.
I use "Dynamic Link Library" (dll) to disable the need of lib.


"Also do you need to create another file just to load/unload an API?"
I think it's more organized to do it that way.

To me it does not make sense to add lines that related to dwm functionality to the main cdoe of the plugin. 

You want me to do that?

"Finally be careful, you add again a lot of tabs, trailing spaces, and various coding style issues (misaligned braces, etc.)."
Okay, yes I forgot to adjust coding style to the requirements. I'll fix it.

Ok do it your way then. :-)
Just fix the coding style, and that should be enough.

Created attachment 372248 [details] [review]
Updated patch

Hello,

I did the coding style correction I found and also logic correction in function doCaptureMagnificationAPI - I added the variable round4Rect in my function...

This correction fixed a bug under function sendBMPToGimp. the bug is that it was never did this (when it called from my function) - 
gimp_layer_resize (layer_id, width, height, 0, 0);
gimp_image_resize (new_image_id, width, height, 0, 0);

These two lines are supposed to do cropping when the width was round up.

So the reason for this border bug was also this and it fixed now..

I tested it on Windows 10 and it is perfect!
It should be noted that this was not tested in Windows 7 and 8 and I can't test it so you need to test it on these operating systems also.

I hope that now it is better :)

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gimp/issues/1458.

Gil > I pushed your patch. Please see my last message on gitlab: https://gitlab.gnome.org/GNOME/gimp/issues/1458#note_227378

Did you make an account on our new gitlab? As you must know by now, we migrated and won't be using bugzilla anymore!