[PATCH] setup: Always draw chooser icons using system colors
Igor Peshansky
pechtcha@cs.nyu.edu
Tue May 23 23:04:00 GMT 2006
On Tue, 23 May 2006, Igor Peshansky wrote:
> On Tue, 23 May 2006, Brian Dessent wrote:
>
> > Igor Peshansky wrote:
> >
> > > As promised in <http://cygwin.com/ml/cygwin/2006-05/msg00612.html>,
> > > here's a patch that makes setup always draw the chooser icons using
> > > the system foreground and background colors (just like it does for
> > > text). I've also taken the opportunity to do a slight bit of code
> > > clean-up. There's still a little glitch left -- when doing a lot of
> > > fast scrolling back and forth, the icons sometimes switch from the
> > > system foreground color to the inverted system background color. I
> > > think I know why this is happening (i.e., the FillRgn() gets ignored
> > > for some reason), but this needs further debugging. As this is a
> > > corner case, I wouldn't worry too much about it. As always, the
> > > ChangeLog is below -- opinions welcome.
> >
> > Thanks for taking a look at this. I like that it abstracts all the
> > blitting to one place, although it seems wasteful to me to have to
> > create and destroy dc_tmp like that every time. However, whatever
> > performance hit this creates is probably better than the blatant
> > incorrectness that is there now, so I guess this is progress. I'm
> > also a little hesitant about this glitch you mention, but it sounds
> > like you're on top of that too so I say go ahead and check this in and
> > we can smooth out the rough edges later.
>
> Right. I was a bit concerned about the performance hit, but I haven't
> noticed any slowdown in drawing the chooser. Also, I couldn't factor out
> (and cache) the creation of those resources, and figured a timely patch
> was better than wasting time exploring it. We can always do this in a
> later refactoring.
I was wrong. I could (and did) factor out the creation of the resources.
My previous approach was trying to create the brush upon initialization,
instead of in the paint() function. There is still no perceptible
performance difference, but there *is* a warm fuzzy feeling.
> As for the glitch, I've only noticed this after very intensive fast
> scrolling back-and-forth, using the scroll buttons instead of the scroll
> bars... It's annoying, but I wouldn't worry about it. What I do worry
> about is that my patch doesn't check the return values of any functions.
> I need to fix that before I check it in. Maybe tonight.
As a bonus, I've also tracked down the glitch, which turned out to be
related. Contrary to the MSDN example code, you *are* supposed to delete
any region you allocate using CreateRectRgn() -- otherwise the system runs
out of space in the region handle table, and refuses to create any more of
them. As I wasn't deleting the regions, eventually new regions didn't get
created, and thus didn't get filled, and thus I lost the neat color combo
that effectively turned the icon into a mask. Adding the DeleteObject()
call fixed my original patch, but now I've factored this out anyway (with
the appropriate deletions for all objects, I think). FWIW, the MSDN entry
on CreateRectRegion is also silent on whether they should be deleted. Oh,
well...
New iteration of the patch attached. The ChangeLog is slightly different,
and is included below. Again, comments welcome, but IMO this is ready to
be checked in (as soon as Brian looks it over and gives the go-ahead). I
would also guess this is a good excuse for generating a new setup
snapshot.
Igor
==============================================================================
ChangeLog:
2006-05-23 Igor Peshansky <pechtcha@cs.nyu.edu>
* PickPackageLine.h (PickPackageLine::DrawIcon): Move to PickView.
* PickView.h (PickView::DrawIcon): Move from PickPackageLine.
(PickView::icon_dc,PickView::bm_icon): New instance field.
(PickView::rect_icon,PickView::bg_fg_brush): Ditto.
* PickCategoryLine.cc (PickCategoryLine::paint): Use
PickView::DrawIcon() instead of BitBlt().
* PickPackageLine.cc (PickPackageLine::DrawIcon): Move to PickView.
(PickPackageLine::paint): Use PickView::DrawIcon().
* PickView.cc (PickView::~PickView): Delete GDI objects.
(PickView::init): Create icon drawing context.
(PickView::DrawIcon): New function. Use system default colors to
draw bitmaps.
(PickView::paint): Set background color instead of using transparent
mode. Create system-colored brush.
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_ pechtcha@cs.nyu.edu | igor@watson.ibm.com
ZZZzz /,`.-'`' -. ;-;;,_ Igor Peshansky, Ph.D. (name changed!)
|,4- ) )-,_. ,\ ( `'-' old name: Igor Pechtchanski
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"
-------------- next part --------------
Index: PickCategoryLine.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickCategoryLine.cc,v
retrieving revision 2.9
diff -u -p -r2.9 PickCategoryLine.cc
--- PickCategoryLine.cc 9 Sep 2005 19:08:02 -0000 2.9
+++ PickCategoryLine.cc 23 May 2006 22:46:47 -0000
@@ -38,9 +38,7 @@ PickCategoryLine::paint (HDC hdc, HRGN h
int by = r + (theView.tm.tmHeight / 2) - 5;
// draw the '+' or '-' box
- SelectObject (theView.bitmap_dc,
- (collapsed ? theView.bm_treeplus : theView.bm_treeminus));
- BitBlt (hdc, x2, by, 11, 11, theView.bitmap_dc, 0, 0, SRCAND);
+ theView.DrawIcon (hdc, x2, by, (collapsed ? theView.bm_treeplus : theView.bm_treeminus));
// draw the category name
TextOut (hdc, x2 + 11 + ICON_MARGIN, r, cat.first.c_str(), cat.first.size());
@@ -52,9 +50,8 @@ PickCategoryLine::paint (HDC hdc, HRGN h
}
// draw the 'spin' glyph
- SelectObject (theView.bitmap_dc, theView.bm_spin);
spin_x = x2 + 11 + ICON_MARGIN + labellength + ICON_MARGIN;
- BitBlt (hdc, spin_x, by, 11, 11, theView.bitmap_dc, 0, 0, SRCAND);
+ theView.DrawIcon (hdc, spin_x, by, theView.bm_spin);
// draw the caption ('Default', 'Install', etc)
TextOut (hdc, spin_x + SPIN_WIDTH + ICON_MARGIN, r,
Index: PickPackageLine.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickPackageLine.cc,v
retrieving revision 2.20
diff -u -p -r2.20 PickPackageLine.cc
--- PickPackageLine.cc 16 Apr 2006 23:07:44 -0000 2.20
+++ PickPackageLine.cc 23 May 2006 22:46:48 -0000
@@ -19,13 +19,6 @@
#include "package_version.h"
void
-PickPackageLine::DrawIcon (HDC hdc, int x, int y, HANDLE hIcon)
-{
- SelectObject (theView.bitmap_dc, hIcon);
- BitBlt (hdc, x, y, 11, 11, theView.bitmap_dc, 0, 0, SRCAND);
-}
-
-void
PickPackageLine::paint (HDC hdc, HRGN unused, int x, int y, int col_num, int show_cat)
{
int rb = y + theView.tm.tmHeight;
@@ -40,21 +33,21 @@ PickPackageLine::paint (HDC hdc, HRGN un
else if (col_num == theView.new_col)
{
// TextOut (hdc, x + HMARGIN/2 + NEW_COL_SIZE_SLOP, y, s.c_str(), s.size());
- // DrawIcon (hdc, x + HMARGIN/2 + ICON_MARGIN/2 + RTARROW_WIDTH, by, theView.bm_spin);
+ // theView.DrawIcon (hdc, x + HMARGIN/2 + ICON_MARGIN/2 + RTARROW_WIDTH, by, theView.bm_spin);
TextOut (hdc, x + HMARGIN/2 + ICON_MARGIN/2 + SPIN_WIDTH , y,
pkg.action_caption ().c_str(), pkg.action_caption ().size());
- DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_spin);
+ theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_spin);
}
else if (col_num == theView.bintick_col)
{
if (/* uninstall or skip */ !pkg.desired ||
/* current version */ pkg.desired == pkg.installed ||
/* no source */ !pkg.desired.accessible())
- DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkna);
+ theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkna);
else if (pkg.desired.picked())
- DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkyes);
+ theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkyes);
else
- DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkno);
+ theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkno);
}
else if (col_num == theView.srctick_col)
{
@@ -73,11 +66,11 @@ PickPackageLine::paint (HDC hdc, HRGN un
#endif
/* when no source mirror available */
!pkg.desired.sourcePackage().accessible())
- DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkna);
+ theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkna);
else if (pkg.desired.sourcePackage().picked())
- DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkyes);
+ theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkyes);
else
- DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkno);
+ theView.DrawIcon (hdc, x + HMARGIN/2, by, theView.bm_checkno);
}
else if (col_num == theView.cat_col)
{
Index: PickPackageLine.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickPackageLine.h,v
retrieving revision 2.6
diff -u -p -r2.6 PickPackageLine.h
--- PickPackageLine.h 21 May 2005 23:04:02 -0000 2.6
+++ PickPackageLine.h 23 May 2006 22:46:48 -0000
@@ -26,7 +26,6 @@ public:
PickPackageLine (PickView &aView, packagemeta & apkg):PickLine (apkg.key), pkg (apkg), theView (aView)
{
};
- void DrawIcon (HDC hdc, int x, int y, HANDLE hIcon);
virtual void paint (HDC hdc, HRGN unused, int x, int y, int col_num, int show_cat);
virtual int click (int const myrow, int const ClickedRow, int const x);
virtual int itemcount () const
Index: PickView.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickView.cc,v
retrieving revision 2.30
diff -u -p -r2.30 PickView.cc
--- PickView.cc 17 Apr 2006 15:40:11 -0000 2.30
+++ PickView.cc 23 May 2006 22:46:48 -0000
@@ -522,6 +522,10 @@ PickView::init(views _mode)
bm_treeplus = LI (IDB_TREE_PLUS);
bm_treeminus = LI (IDB_TREE_MINUS);
#undef LI
+ icon_dc = CreateCompatibleDC (dc);
+ bm_icon = CreateCompatibleBitmap (dc, 11, 11);
+ SelectObject (icon_dc, bm_icon);
+ rect_icon = CreateRectRgn (0, 0, 11, 11);
row_height = (tm.tmHeight + tm.tmExternalLeading + ROW_MARGIN);
int irh = tm.tmExternalLeading + tm.tmDescent + 11 + ROW_MARGIN;
@@ -575,6 +579,16 @@ PickView::init(views _mode)
PickView::~PickView()
{
+ DeleteDC (bitmap_dc);
+ DeleteObject (bm_spin);
+ DeleteObject (bm_checkyes);
+ DeleteObject (bm_checkno);
+ DeleteObject (bm_checkna);
+ DeleteObject (bm_treeplus);
+ DeleteObject (bm_treeminus);
+ DeleteObject (rect_icon);
+ DeleteObject (bm_icon);
+ DeleteDC (icon_dc);
}
bool PickView::registerWindowClass ()
@@ -798,6 +812,28 @@ PickView::WindowProc (UINT message, WPAR
return DefWindowProc (GetHWND(), message, wParam, lParam);
}
+////
+// Turn black into foreground color and white into background color by
+// 1) Filling a square with ~(FG^BG)
+// 2) Blitting the bitmap on it with NOTSRCERASE (white->black; black->FG^BG)
+// 3) Blitting the result on BG with SRCINVERT (white->BG; black->FG)
+void
+PickView::DrawIcon (HDC hdc, int x, int y, HANDLE hIcon)
+{
+ SelectObject (bitmap_dc, hIcon);
+ FillRgn (icon_dc, rect_icon, bg_fg_brush);
+ BitBlt (icon_dc, 0, 0, 11, 11, bitmap_dc, 0, 0, NOTSRCERASE);
+ BitBlt (hdc, x, y, 11, 11, icon_dc, 0, 0, SRCINVERT);
+///////////// On WinNT-based systems, we could've done the below instead
+///////////// See http://support.microsoft.com/default.aspx?scid=kb;en-us;79212
+// SelectObject (hdc, GetSysColorBrush (COLOR_WINDOWTEXT));
+// HBITMAP bm_icon = CreateBitmap (11, 11, 1, 1, NULL);
+// SelectObject (icon_dc, bm_icon);
+// BitBlt (icon_dc, 0, 0, 11, 11, bitmap_dc, 0, 0, SRCCOPY);
+// MaskBlt (hdc, x2, by, 11, 11, bitmap_dc, 0, 0, bm_icon, 0, 0, MAKEROP4 (SRCAND, PATCOPY));
+// DeleteObject (bm_icon);
+}
+
void
PickView::paint (HWND hwnd)
{
@@ -821,9 +857,13 @@ PickView::paint (HWND hwnd)
SelectObject (hdc, sysfont);
SetTextColor (hdc, GetSysColor (COLOR_WINDOWTEXT));
- SetBkMode (hdc, TRANSPARENT);
+ SetBkColor (hdc, GetSysColor (COLOR_WINDOW));
FillRgn (hdc, hUpdRgn, GetSysColorBrush(COLOR_WINDOW));
+ COLORREF clr = ~GetSysColor (COLOR_WINDOW) ^ GetSysColor (COLOR_WINDOWTEXT);
+ clr = RGB (GetRValue (clr), GetGValue (clr), GetBValue (clr)); // reconvert
+ bg_fg_brush = CreateSolidBrush (clr);
+
RECT cr;
::GetClientRect (hwnd, &cr);
@@ -842,6 +882,7 @@ PickView::paint (HWND hwnd)
}
DeleteObject (hUpdRgn);
+ DeleteObject (bg_fg_brush);
EndPaint (hwnd, &ps);
}
Index: PickView.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickView.h,v
retrieving revision 2.16
diff -u -p -r2.16 PickView.h
--- PickView.h 17 Apr 2006 15:40:11 -0000 2.16
+++ PickView.h 23 May 2006 22:46:48 -0000
@@ -45,6 +45,7 @@ public:
void defaultTrust (trusts trust);
void cycleViewMode ();
void setViewMode (views mode);
+ void DrawIcon (HDC hdc, int x, int y, HANDLE hIcon);
void paint (HWND hwnd);
LRESULT CALLBACK list_click (HWND hwnd, BOOL dblclk, int x, int y, UINT hitCode);
LRESULT CALLBACK list_hscroll (HWND hwnd, HWND hctl, UINT code, int pos);
@@ -70,7 +71,10 @@ public:
int last_col;
int row_height;
TEXTMETRIC tm;
- HDC bitmap_dc;
+ HDC bitmap_dc, icon_dc;
+ HBITMAP bm_icon;
+ HRGN rect_icon;
+ HBRUSH bg_fg_brush;
HANDLE bm_spin, bm_checkyes, bm_checkno, bm_checkna, bm_treeplus, bm_treeminus;
trusts deftrust;
HANDLE sysfont;
More information about the Cygwin-apps
mailing list