* 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 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
* 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
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).