unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
@ 2011-12-29 14:05 Daniel Colascione
  2011-12-29 16:13 ` Juanma Barranquero
  2011-12-30  1:04 ` Jason Rumney
  0 siblings, 2 replies; 22+ messages in thread
From: Daniel Colascione @ 2011-12-29 14:05 UTC (permalink / raw)
  To: 10397

Under remote desktop, Windows returns the wrong number of colors from
GetDeviceCaps (hdc, NUMCOLORS).  I hit this bug myself, and MSDN
comments seem to indicate that others hit it as well.  The workaround
seems harmless: on non-palettized displays, calculating the number of
display colors based on display bitness should produce good results.
---
 src/w32fns.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/w32fns.c b/src/w32fns.c
index 822e353..4b94f16 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -4510,7 +4510,10 @@ If omitted or nil, that stands for the selected frame's display.  */)
   if (dpyinfo->has_palette)
     cap = GetDeviceCaps (hdc, SIZEPALETTE);
   else
-    cap = GetDeviceCaps (hdc, NUMCOLORS);
+    // GetDeviceCaps (NUMCOLORS) is buggy under remote desktop and sometimes
+    // returns the number of system reserved colors (20) instead of
+    // the actual number of available colors.
+    cap = -1;
 
   /* We force 24+ bit depths to 24-bit, both to prevent an overflow
      and because probably is more meaningful on Windows anyway */
-- 
1.7.5.1






^ permalink raw reply related	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 14:05 bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround Daniel Colascione
@ 2011-12-29 16:13 ` Juanma Barranquero
  2011-12-29 16:23   ` Daniel Colascione
  2011-12-30  1:04 ` Jason Rumney
  1 sibling, 1 reply; 22+ messages in thread
From: Juanma Barranquero @ 2011-12-29 16:13 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 10397

On Thu, Dec 29, 2011 at 15:05, Daniel Colascione <dancol@dancol.org> wrote:

> The workaround
> seems harmless: on non-palettized displays, calculating the number of
> display colors based on display bitness should produce good results.

Even so, why fix what is not broken? Why can't you just do

=== modified file 'src/w32fns.c'
--- src/w32fns.c	2011-12-04 08:02:42 +0000
+++ src/w32fns.c	2011-12-29 16:10:33 +0000
@@ -4511,5 +4511,12 @@
     cap = GetDeviceCaps (hdc, SIZEPALETTE);
   else
-    cap = GetDeviceCaps (hdc, NUMCOLORS);
+    {
+      cap = GetDeviceCaps (hdc, NUMCOLORS);
+      /* GetDeviceCaps (NUMCOLORS) is buggy under remote desktop and
+         sometimes returns the number of system reserved colors (20)
+         instead of the actual number of available colors.  */
+      if (cap == 20)
+        cap = -1;
+    }

   /* We force 24+ bit depths to 24-bit, both to prevent an overflow

> +    // GetDeviceCaps (NUMCOLORS) is buggy under remote desktop and sometimes
> +    // returns the number of system reserved colors (20) instead of
> +    // the actual number of available colors.

Please, don't use "C++ / modern C" style comments; use /* */ instead.

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 16:13 ` Juanma Barranquero
@ 2011-12-29 16:23   ` Daniel Colascione
  2011-12-29 16:27     ` Juanma Barranquero
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Colascione @ 2011-12-29 16:23 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: 10397

[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]

On 12/29/11 8:13 AM, Juanma Barranquero wrote:
> On Thu, Dec 29, 2011 at 15:05, Daniel Colascione <dancol@dancol.org> wrote:
> 
>> The workaround
>> seems harmless: on non-palettized displays, calculating the number of
>> display colors based on display bitness should produce good results.
> 
> Even so, why fix what is not broken? Why can't you just do
> 
> === modified file 'src/w32fns.c'
> --- src/w32fns.c	2011-12-04 08:02:42 +0000
> +++ src/w32fns.c	2011-12-29 16:10:33 +0000
> @@ -4511,5 +4511,12 @@
>      cap = GetDeviceCaps (hdc, SIZEPALETTE);
>    else
> -    cap = GetDeviceCaps (hdc, NUMCOLORS);
> +    {
> +      cap = GetDeviceCaps (hdc, NUMCOLORS);
> +      /* GetDeviceCaps (NUMCOLORS) is buggy under remote desktop and
> +         sometimes returns the number of system reserved colors (20)
> +         instead of the actual number of available colors.  */
> +      if (cap == 20)
> +        cap = -1;
> +    }

Why? What's the point of adding the extra complexity?
Setting cap to -1 leads to this line

    1 << min (dpyinfo->n_planes * dpyinfo->n_cbits, 24);

which produces a reasonable result for direct color displays.
Why keep using NUMCOLORS, which we know to be broken?

>    /* We force 24+ bit depths to 24-bit, both to prevent an overflow
> 
>> +    // GetDeviceCaps (NUMCOLORS) is buggy under remote desktop and sometimes
>> +    // returns the number of system reserved colors (20) instead of
>> +    // the actual number of available colors.
> 
> Please, don't use "C++ / modern C" style comments; use /* */ instead.
> 

That block slipped through.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 16:23   ` Daniel Colascione
@ 2011-12-29 16:27     ` Juanma Barranquero
  2011-12-29 16:42       ` Daniel Colascione
  0 siblings, 1 reply; 22+ messages in thread
From: Juanma Barranquero @ 2011-12-29 16:27 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 10397

On Thu, Dec 29, 2011 at 17:23, Daniel Colascione <dancol@dancol.org> wrote:

> Why? What's the point of adding the extra complexity?
> Setting cap to -1 leads to this line
>
>    1 << min (dpyinfo->n_planes * dpyinfo->n_cbits, 24);
>
> which produces a reasonable result for direct color displays.
> Why keep using NUMCOLORS, which we know to be broken?

No, you said that NUMCOLORS is known to be broken in a very specific
case. In the general, non-RemoteDektop case, it works. Can you
guarantee that for every non-palettized display, it will produce the
same number? Because otherwise you're changing the current behavior
for people who's not affected by the RemoteDeskopt-related bug.

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 16:27     ` Juanma Barranquero
@ 2011-12-29 16:42       ` Daniel Colascione
  2011-12-29 16:45         ` Juanma Barranquero
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Colascione @ 2011-12-29 16:42 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: 10397

[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]

On 12/29/11 8:27 AM, Juanma Barranquero wrote:
> On Thu, Dec 29, 2011 at 17:23, Daniel Colascione <dancol@dancol.org> wrote:
> 
>> Why? What's the point of adding the extra complexity?
>> Setting cap to -1 leads to this line
>>
>>    1 << min (dpyinfo->n_planes * dpyinfo->n_cbits, 24);
>>
>> which produces a reasonable result for direct color displays.
>> Why keep using NUMCOLORS, which we know to be broken?
> 
> No, you said that NUMCOLORS is known to be broken in a very specific
> case. In the general, non-RemoteDektop case, it works. 

I'm not convinced there aren't other bugs lurking in the code backing
NUMCOLORS; after all, it's doing the same simple calculation we are, and
it's somehow doing it wrong.

http://msdn.microsoft.com/en-us/library/dd144877%28v=VS.85%29.aspx#3
suggests that NUMCOLORS is generally flaky.

> Can you
> guarantee that for every non-palettized display, it will produce the
> same number? Because otherwise you're changing the current behavior
> for people who's not affected by the RemoteDeskopt-related bug.

No, I can't guarantee that my original change always produces the same
results: it might fix other bugs.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 16:42       ` Daniel Colascione
@ 2011-12-29 16:45         ` Juanma Barranquero
  2011-12-29 22:59           ` Daniel Colascione
  0 siblings, 1 reply; 22+ messages in thread
From: Juanma Barranquero @ 2011-12-29 16:45 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 10397

On Thu, Dec 29, 2011 at 17:42, Daniel Colascione <dancol@dancol.org> wrote:

> I'm not convinced there aren't other bugs lurking in the code backing
> NUMCOLORS; after all, it's doing the same simple calculation we are, and
> it's somehow doing it wrong.

You have also no proof. I don't see why should we try to fix bugs we
don't even know are real. Better to fiix the one you know it *is*
real, IMO.

> No, I can't guarantee that my original change always produces the same
> results: it might fix

or introduce

> other bugs.

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 16:45         ` Juanma Barranquero
@ 2011-12-29 22:59           ` Daniel Colascione
  2011-12-29 23:10             ` Juanma Barranquero
  2011-12-30  3:07             ` Juanma Barranquero
  0 siblings, 2 replies; 22+ messages in thread
From: Daniel Colascione @ 2011-12-29 22:59 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: 10397

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

On 12/29/11 8:45 AM, Juanma Barranquero wrote:
> On Thu, Dec 29, 2011 at 17:42, Daniel Colascione <dancol@dancol.org> wrote:
> 
>> I'm not convinced there aren't other bugs lurking in the code backing
>> NUMCOLORS; after all, it's doing the same simple calculation we are, and
>> it's somehow doing it wrong.
> 
> You have also no proof. I don't see why should we try to fix bugs we
> don't even know are real. Better to fiix the one you know it *is*
> real, IMO.

What about this: we'll distrust any NUMCOLORS response less than 256.
You'll never use direct color with a bit depth that small, so any answer
in that range must be bogus.  This approach would address my concerns
about wrong values other than exactly 20 being returned.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 22:59           ` Daniel Colascione
@ 2011-12-29 23:10             ` Juanma Barranquero
  2011-12-29 23:13               ` Daniel Colascione
  2011-12-30  3:07             ` Juanma Barranquero
  1 sibling, 1 reply; 22+ messages in thread
From: Juanma Barranquero @ 2011-12-29 23:10 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 10397

On Thu, Dec 29, 2011 at 23:59, Daniel Colascione <dancol@dancol.org> wrote:

> What about this: we'll distrust any NUMCOLORS response less than 256.
> You'll never use direct color with a bit depth that small, so any answer
> in that range must be bogus.  This approach would address my concerns
> about wrong values other than exactly 20 being returned.

OK, assuming you're talking post-24.1.

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 23:10             ` Juanma Barranquero
@ 2011-12-29 23:13               ` Daniel Colascione
  2011-12-29 23:18                 ` Juanma Barranquero
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Colascione @ 2011-12-29 23:13 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: 10397

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

On 12/29/11 3:10 PM, Juanma Barranquero wrote:
> On Thu, Dec 29, 2011 at 23:59, Daniel Colascione <dancol@dancol.org> wrote:
> 
>> What about this: we'll distrust any NUMCOLORS response less than 256.
>> You'll never use direct color with a bit depth that small, so any answer
>> in that range must be bogus.  This approach would address my concerns
>> about wrong values other than exactly 20 being returned.
> 
> OK, assuming you're talking post-24.1.
> 
>     Juanma
> 

Would it be possible to get the more limited (exactly 20) fix into 24.1?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 23:13               ` Daniel Colascione
@ 2011-12-29 23:18                 ` Juanma Barranquero
  0 siblings, 0 replies; 22+ messages in thread
From: Juanma Barranquero @ 2011-12-29 23:18 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 10397

On Fri, Dec 30, 2011 at 00:13, Daniel Colascione <dancol@dancol.org> wrote:

> Would it be possible to get the more limited (exactly 20) fix into 24.1?

The "20-check" is a bug fix against a specific bug. OOH, it's not a
regression, OTOH, its impact is limited to Windows. You'll have to ask
Chong and/or Stefan; it's their decision.

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 14:05 bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround Daniel Colascione
  2011-12-29 16:13 ` Juanma Barranquero
@ 2011-12-30  1:04 ` Jason Rumney
  2011-12-30  1:10   ` Daniel Colascione
  1 sibling, 1 reply; 22+ messages in thread
From: Jason Rumney @ 2011-12-30  1:04 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 10397

Daniel Colascione <dancol@dancol.org> writes:

> Under remote desktop, Windows returns the wrong number of colors from
> GetDeviceCaps (hdc, NUMCOLORS).  I hit this bug myself, and MSDN
> comments seem to indicate that others hit it as well.  The workaround
> seems harmless: on non-palettized displays, calculating the number of
> display colors based on display bitness should produce good results.

I've always been under the impression that this is deliberate, and
related to the bandwidth that is available, so at least applications
that want to improve performance over low bandwidth links can restrict
their use of colors.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-30  1:04 ` Jason Rumney
@ 2011-12-30  1:10   ` Daniel Colascione
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Colascione @ 2011-12-30  1:10 UTC (permalink / raw)
  To: Jason Rumney; +Cc: 10397

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

On 12/29/11 5:04 PM, Jason Rumney wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
>> Under remote desktop, Windows returns the wrong number of colors from
>> GetDeviceCaps (hdc, NUMCOLORS).  I hit this bug myself, and MSDN
>> comments seem to indicate that others hit it as well.  The workaround
>> seems harmless: on non-palettized displays, calculating the number of
>> display colors based on display bitness should produce good results.
> 
> I've always been under the impression that this is deliberate, and
> related to the bandwidth that is available, so at least applications
> that want to improve performance over low bandwidth links can restrict
> their use of colors.

A remote desktop user can change the depth of the virtual display
presented to applications on the server.  If a user wants to trade
fidelity for bandwidth, he can configure his client to use an 8bpp
visual.  Some users (me) configure their clients for a relatively high
bit depth, but find that the OS lies to applications some of the time
(NUMCOLORS is wrong, but the display bitness is accurate).  I think the
NUMCOLORS behavior is a real bug; if it weren't, the lie would be more
consistently presented.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-29 22:59           ` Daniel Colascione
  2011-12-29 23:10             ` Juanma Barranquero
@ 2011-12-30  3:07             ` Juanma Barranquero
  2011-12-30  3:18               ` Daniel Colascione
  1 sibling, 1 reply; 22+ messages in thread
From: Juanma Barranquero @ 2011-12-30  3:07 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 10397

On Thu, Dec 29, 2011 at 23:59, Daniel Colascione <dancol@dancol.org> wrote:

> What about this: we'll distrust any NUMCOLORS response less than 256.
> You'll never use direct color with a bit depth that small, so any answer
> in that range must be bogus.

Hmm. Shouldn't in fact GetDeviceCaps (hdc, NUMCOLORS) always be <= 256?

According to http://msdn.microsoft.com/en-us/library/dd144877(v=vs.85).aspx

  NUMCOLORS
  Number of entries in the device's color table, if the device has a
color depth of no more than 8 bits per pixel. For devices with greater
color depths, 1 is returned.

(It says "1", but it's a typo for "-1".)

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-30  3:07             ` Juanma Barranquero
@ 2011-12-30  3:18               ` Daniel Colascione
  2011-12-30  8:51                 ` Eli Zaretskii
  2011-12-30  9:00                 ` Eli Zaretskii
  0 siblings, 2 replies; 22+ messages in thread
From: Daniel Colascione @ 2011-12-30  3:18 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: 10397

[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]

On 12/29/11 7:07 PM, Juanma Barranquero wrote:
> On Thu, Dec 29, 2011 at 23:59, Daniel Colascione <dancol@dancol.org> wrote:
> 
>> What about this: we'll distrust any NUMCOLORS response less than 256.
>> You'll never use direct color with a bit depth that small, so any answer
>> in that range must be bogus.
> 
> Hmm. Shouldn't in fact GetDeviceCaps (hdc, NUMCOLORS) always be <= 256?
> 
> According to http://msdn.microsoft.com/en-us/library/dd144877(v=vs.85).aspx
> 
>   NUMCOLORS
>   Number of entries in the device's color table, if the device has a
> color depth of no more than 8 bits per pixel. For devices with greater
> color depths, 1 is returned.
> 
> (It says "1", but it's a typo for "-1".)

Good catch. What about this (untested) code?

  hdc = GetDC (dpyinfo->root_window);
  if (dpyinfo->has_palette)
    cap = GetDeviceCaps (hdc, SIZEPALETTE);
  else if (dpyinfo->n_cbits <= 8)
    /* According to the MSDN, GetDeviceCaps (NUMCOLORS) is valid only
       for devices with at most eight bits per pixel.  It's supposed
       to return -1 for other displays, but because it actually
       returns other, incorrect values under some conditions (e.g.,
       remote desktop), only use it when we know it's valid.  */
    cap = GetDeviceCaps (hdc, NUMCOLORS);
  else
    cap = -1;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-30  3:18               ` Daniel Colascione
@ 2011-12-30  8:51                 ` Eli Zaretskii
  2011-12-30  9:00                 ` Eli Zaretskii
  1 sibling, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2011-12-30  8:51 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: lekktu, 10397

> Date: Thu, 29 Dec 2011 19:18:13 -0800
> From: Daniel Colascione <dancol@dancol.org>
> Cc: 10397@debbugs.gnu.org
> 
> > Hmm. Shouldn't in fact GetDeviceCaps (hdc, NUMCOLORS) always be <= 256?
> > 
> > According to http://msdn.microsoft.com/en-us/library/dd144877(v=vs.85).aspx
> > 
> >   NUMCOLORS
> >   Number of entries in the device's color table, if the device has a
> > color depth of no more than 8 bits per pixel. For devices with greater
> > color depths, 1 is returned.
> > 
> > (It says "1", but it's a typo for "-1".)
> 
> Good catch. What about this (untested) code?
> 
>   hdc = GetDC (dpyinfo->root_window);
>   if (dpyinfo->has_palette)
>     cap = GetDeviceCaps (hdc, SIZEPALETTE);
>   else if (dpyinfo->n_cbits <= 8)
>     /* According to the MSDN, GetDeviceCaps (NUMCOLORS) is valid only
>        for devices with at most eight bits per pixel.  It's supposed
>        to return -1 for other displays, but because it actually
>        returns other, incorrect values under some conditions (e.g.,
>        remote desktop), only use it when we know it's valid.  */
>     cap = GetDeviceCaps (hdc, NUMCOLORS);
>   else
>     cap = -1;

There's a comment near the end of the GetDeviceCaps documentation
saying this:

  NUMCOLORS doesn't always return one with more than 256 colors

  The documentation for NUMCOLORS is wrong, devices that support more
  than 256 colors often return -1 and some virtual machines can return
  the number of Windows reserved colors (i.e. 20), even in high color
  mode.  Combine the NUMCOLORS return value with BITSPIXEL and PLANES
  to reliably detect indexed color.

So how about using `1 << (n_planes * n_cbits)' (which we compute when
NUMCOLORS call returns -1) unconditionally?  IOW, why do we need to
call GetDeviceCaps(..., NUMCOLORS) here in the first place?





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-30  3:18               ` Daniel Colascione
  2011-12-30  8:51                 ` Eli Zaretskii
@ 2011-12-30  9:00                 ` Eli Zaretskii
  2011-12-30 12:24                   ` Juanma Barranquero
  1 sibling, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2011-12-30  9:00 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: lekktu, 10397

> Date: Thu, 29 Dec 2011 19:18:13 -0800
> From: Daniel Colascione <dancol@dancol.org>
> Cc: 10397@debbugs.gnu.org
> 
> >   NUMCOLORS
> >   Number of entries in the device's color table, if the device has a
> > color depth of no more than 8 bits per pixel. For devices with greater
> > color depths, 1 is returned.
> > 
> > (It says "1", but it's a typo for "-1".)
> 
> Good catch. What about this (untested) code?
> 
>   hdc = GetDC (dpyinfo->root_window);
>   if (dpyinfo->has_palette)
>     cap = GetDeviceCaps (hdc, SIZEPALETTE);
>   else if (dpyinfo->n_cbits <= 8)
>     /* According to the MSDN, GetDeviceCaps (NUMCOLORS) is valid only
>        for devices with at most eight bits per pixel.  It's supposed
>        to return -1 for other displays, but because it actually
>        returns other, incorrect values under some conditions (e.g.,
>        remote desktop), only use it when we know it's valid.  */
>     cap = GetDeviceCaps (hdc, NUMCOLORS);
>   else
>     cap = -1;

Looks good to me.  I think this could go into 24.1, unless Jason
(or someone else) objects.

As I wrote elsewhere, past 24.1, we could explore the possibility of
not calling GetDeviceCaps here at all (assuming that using the number
of planes and bits per plane does the job even when the latter is 8 or
less).





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-30  9:00                 ` Eli Zaretskii
@ 2011-12-30 12:24                   ` Juanma Barranquero
  2012-08-07 17:13                     ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Juanma Barranquero @ 2011-12-30 12:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 10397

On Fri, Dec 30, 2011 at 10:00, Eli Zaretskii <eliz@gnu.org> wrote:

> Looks good to me.  I think this could go into 24.1, unless Jason
> (or someone else) objects.
>
> As I wrote elsewhere, past 24.1, we could explore the possibility of
> not calling GetDeviceCaps here at all (assuming that using the number
> of planes and bits per plane does the job even when the latter is 8 or
> less).

Agreed.

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2011-12-30 12:24                   ` Juanma Barranquero
@ 2012-08-07 17:13                     ` Eli Zaretskii
  2012-08-07 17:33                       ` Daniel Colascione
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2012-08-07 17:13 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: Juanma Barranquero, 10397

> Date: Tue, 07 Aug 2012 01:19:27 -0700
> From: Daniel Colascione <dancol@dancol.org>
> 
> Under remote desktop, Windows returns the wrong number of colors from
> GetDeviceCaps (hdc, NUMCOLORS).  I hit this bug myself, and MSDN
> comments seem to indicate that others hit it as well.  The workaround
> seems harmless: on non-palettized displays, calculating the number of
> display colors based on display bitness should produce good results.
> ---
>  src/w32fns.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/src/w32fns.c b/src/w32fns.c
> index b82d4bc..7fc5cf5 100644
> --- a/src/w32fns.c
> +++ b/src/w32fns.c
> @@ -4513,8 +4513,15 @@ If omitted or nil, that stands for the selected frame's display.  */)
>    hdc = GetDC (dpyinfo->root_window);
>    if (dpyinfo->has_palette)
>      cap = GetDeviceCaps (hdc, SIZEPALETTE);
> -  else
> +  else if (dpyinfo->n_cbits <= 8)
> +    /* According to the MSDN, GetDeviceCaps (NUMCOLORS) is valid only
> +       for devices with at most eight bits per pixel.  It's supposed
> +       to return -1 for other displays, but because it actually
> +       returns other, incorrect values under some conditions (e.g.,
> +       remote desktop), only use it when we know it's valid.  */
>      cap = GetDeviceCaps (hdc, NUMCOLORS);
> +  else
> +    cap = -1;
>  
>    /* We force 24+ bit depths to 24-bit, both to prevent an overflow
>       and because probably is more meaningful on Windows anyway */

Last time we spoke, the conclusion (at least mine ;-) was that it
might be better not to call GetDeviceCaps at all, and instead reuse
the code below this, which uses the number of planes and bits per
plane.  If you agree with that reasoning, could you please see if that
change solves your problem?

In any case, let's separate between this patch and the rest of "w32
GUI with Cygwin" patches, as they are really unrelated.  In
particular, as soon as we agree on this one, you cam go ahead and
commit it regardless of the rest.

Thanks.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2012-08-07 17:13                     ` Eli Zaretskii
@ 2012-08-07 17:33                       ` Daniel Colascione
  2012-08-07 18:07                         ` Eli Zaretskii
  2016-02-25  6:25                         ` Lars Ingebrigtsen
  0 siblings, 2 replies; 22+ messages in thread
From: Daniel Colascione @ 2012-08-07 17:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Juanma Barranquero, 10397

[-- Attachment #1: Type: text/plain, Size: 2307 bytes --]

On 8/7/12 10:13 AM, Eli Zaretskii wrote:
>> Date: Tue, 07 Aug 2012 01:19:27 -0700
>> From: Daniel Colascione <dancol@dancol.org>
>>
>> Under remote desktop, Windows returns the wrong number of colors from
>> GetDeviceCaps (hdc, NUMCOLORS).  I hit this bug myself, and MSDN
>> comments seem to indicate that others hit it as well.  The workaround
>> seems harmless: on non-palettized displays, calculating the number of
>> display colors based on display bitness should produce good results.
>> ---
>>  src/w32fns.c |    9 ++++++++-
>>  1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/w32fns.c b/src/w32fns.c
>> index b82d4bc..7fc5cf5 100644
>> --- a/src/w32fns.c
>> +++ b/src/w32fns.c
>> @@ -4513,8 +4513,15 @@ If omitted or nil, that stands for the selected frame's display.  */)
>>    hdc = GetDC (dpyinfo->root_window);
>>    if (dpyinfo->has_palette)
>>      cap = GetDeviceCaps (hdc, SIZEPALETTE);
>> -  else
>> +  else if (dpyinfo->n_cbits <= 8)
>> +    /* According to the MSDN, GetDeviceCaps (NUMCOLORS) is valid only
>> +       for devices with at most eight bits per pixel.  It's supposed
>> +       to return -1 for other displays, but because it actually
>> +       returns other, incorrect values under some conditions (e.g.,
>> +       remote desktop), only use it when we know it's valid.  */
>>      cap = GetDeviceCaps (hdc, NUMCOLORS);
>> +  else
>> +    cap = -1;
>>  
>>    /* We force 24+ bit depths to 24-bit, both to prevent an overflow
>>       and because probably is more meaningful on Windows anyway */
> 
> Last time we spoke, the conclusion (at least mine ;-) was that it
> might be better not to call GetDeviceCaps at all, and instead reuse
> the code below this, which uses the number of planes and bits per
> plane.  If you agree with that reasoning, could you please see if that
> change solves your problem?

Sorry about that --- I'm bringing a lot of this up form very cold
mental storage. It's been a little while since I've had a chance to do
any Emacs hacking.

I'm perfectly happy using the planes-and-bits code instead of calling
GetDeviceCaps. I'll remove this patch from the cygw32 changeset and
check the (now, much simpler) fix for the colors issue into the trunk,
if that's all right.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2012-08-07 17:33                       ` Daniel Colascione
@ 2012-08-07 18:07                         ` Eli Zaretskii
  2016-02-25  6:25                         ` Lars Ingebrigtsen
  1 sibling, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2012-08-07 18:07 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: lekktu, 10397

> Date: Tue, 07 Aug 2012 10:33:23 -0700
> From: Daniel Colascione <dancol@dancol.org>
> CC: Juanma Barranquero <lekktu@gmail.com>, 10397@debbugs.gnu.org
> 
> > Last time we spoke, the conclusion (at least mine ;-) was that it
> > might be better not to call GetDeviceCaps at all, and instead reuse
> > the code below this, which uses the number of planes and bits per
> > plane.  If you agree with that reasoning, could you please see if that
> > change solves your problem?
> 
> Sorry about that --- I'm bringing a lot of this up form very cold
> mental storage. It's been a little while since I've had a chance to do
> any Emacs hacking.

No need to apologize.  It's not that my memory is better than yours, I
just looked up the bug report and re-read the entire thread...

> I'm perfectly happy using the planes-and-bits code instead of calling
> GetDeviceCaps. I'll remove this patch from the cygw32 changeset and
> check the (now, much simpler) fix for the colors issue into the trunk,
> if that's all right.

Please go ahead, but I hope you have a way to check the change you
will commit with remote desktop, because I don't.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2012-08-07 17:33                       ` Daniel Colascione
  2012-08-07 18:07                         ` Eli Zaretskii
@ 2016-02-25  6:25                         ` Lars Ingebrigtsen
  2016-12-13  0:04                           ` Glenn Morris
  1 sibling, 1 reply; 22+ messages in thread
From: Lars Ingebrigtsen @ 2016-02-25  6:25 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: Juanma Barranquero, 10397

Daniel Colascione <dancol@dancol.org> writes:

>>>    hdc = GetDC (dpyinfo->root_window);
>>>    if (dpyinfo->has_palette)
>>>      cap = GetDeviceCaps (hdc, SIZEPALETTE);
>>> -  else
>>> +  else if (dpyinfo->n_cbits <= 8)
>>> +    /* According to the MSDN, GetDeviceCaps (NUMCOLORS) is valid only
>>> +       for devices with at most eight bits per pixel.  It's supposed
>>> +       to return -1 for other displays, but because it actually
>>> +       returns other, incorrect values under some conditions (e.g.,
>>> +       remote desktop), only use it when we know it's valid.  */
>>>      cap = GetDeviceCaps (hdc, NUMCOLORS);
>>> +  else
>>> +    cap = -1;

Looking at w32fns.c, I can't really find anything that resembles the
surrounding code in this patch.  Instead there seems to be newer code
that does...  stuff...  to palettes.

So is this all outdated now, and this stuff works fine?
-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround
  2016-02-25  6:25                         ` Lars Ingebrigtsen
@ 2016-12-13  0:04                           ` Glenn Morris
  0 siblings, 0 replies; 22+ messages in thread
From: Glenn Morris @ 2016-12-13  0:04 UTC (permalink / raw)
  To: 10397-done

Version: 24.3

I see this was fixed in emacs-24.2-3079-g821812e.






^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-12-13  0:04 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-29 14:05 bug#10397: [PATCH] Under Remote Desktop, NUMCOLORS is unreliable; workaround Daniel Colascione
2011-12-29 16:13 ` Juanma Barranquero
2011-12-29 16:23   ` Daniel Colascione
2011-12-29 16:27     ` Juanma Barranquero
2011-12-29 16:42       ` Daniel Colascione
2011-12-29 16:45         ` Juanma Barranquero
2011-12-29 22:59           ` Daniel Colascione
2011-12-29 23:10             ` Juanma Barranquero
2011-12-29 23:13               ` Daniel Colascione
2011-12-29 23:18                 ` Juanma Barranquero
2011-12-30  3:07             ` Juanma Barranquero
2011-12-30  3:18               ` Daniel Colascione
2011-12-30  8:51                 ` Eli Zaretskii
2011-12-30  9:00                 ` Eli Zaretskii
2011-12-30 12:24                   ` Juanma Barranquero
2012-08-07 17:13                     ` Eli Zaretskii
2012-08-07 17:33                       ` Daniel Colascione
2012-08-07 18:07                         ` Eli Zaretskii
2016-02-25  6:25                         ` Lars Ingebrigtsen
2016-12-13  0:04                           ` Glenn Morris
2011-12-30  1:04 ` Jason Rumney
2011-12-30  1:10   ` Daniel Colascione

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).