From be01929f5d00456024e203bc56955105930e248c Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Tue, 14 May 2019 18:18:06 +0200 Subject: ui: fix dpi related code smells Signed-off-by: Alexander Neumann --- ui/aboutdialog.go | 8 +++---- ui/confview.go | 30 ++++++------------------ ui/editdialog.go | 2 +- ui/iconprovider.go | 67 ++++++++++++++++-------------------------------------- ui/listview.go | 2 +- ui/managewindow.go | 4 ++-- ui/tray.go | 16 ++++++------- ui/tunnelspage.go | 36 +++++++++++++---------------- ui/updatepage.go | 8 +++---- 9 files changed, 61 insertions(+), 112 deletions(-) diff --git a/ui/aboutdialog.go b/ui/aboutdialog.go index 1e1c3c1c..8109b159 100644 --- a/ui/aboutdialog.go +++ b/ui/aboutdialog.go @@ -27,7 +27,7 @@ func onAbout(owner walk.Form) { dlg, _ := walk.NewDialogWithFixedSize(owner) dlg.SetTitle("About WireGuard") dlg.SetLayout(vbl) - if icon, err := loadLogoIcon(dlg.DPI() / 3); err == nil { //TODO: calculate DPI dynamically + if icon, err := loadLogoIcon(32); err == nil { dlg.SetIcon(icon) } @@ -40,18 +40,18 @@ func onAbout(owner walk.Form) { if button == walk.LeftButton { win.ShellExecute(dlg.Handle(), nil, windows.StringToUTF16Ptr("https://www.wireguard.com/"), nil, nil, win.SW_SHOWNORMAL) } else if easterEggIndex >= 0 && button == walk.RightButton { - if icon, err := loadSystemIcon("moricons", int32(easterEggIndex), dlg.DPI()*4/3); err == nil { //TODO: calculate DPI dynamically + if icon, err := loadSystemIcon("moricons", int32(easterEggIndex), 128); err == nil { iv.SetImage(icon) easterEggIndex++ } else { easterEggIndex = -1 - if logo, err := loadLogoIcon(dlg.DPI() * 4 / 3); err == nil { //TODO: calculate DPI dynamically + if logo, err := loadLogoIcon(128); err == nil { iv.SetImage(logo) } } } }) - if logo, err := loadLogoIcon(dlg.DPI() * 4 / 3); err == nil { //TODO: calculate DPI dynamically + if logo, err := loadLogoIcon(128); err == nil { iv.SetImage(logo) } diff --git a/ui/confview.go b/ui/confview.go index 869ba229..1024befa 100644 --- a/ui/confview.go +++ b/ui/confview.go @@ -36,7 +36,6 @@ type labelStatusLine struct { statusComposite *walk.Composite statusImage *walk.ImageView statusLabel *walk.LineEdit - cachedImages map[rectAndSizeAndState]walk.Image } type labelTextLine struct { @@ -86,27 +85,13 @@ func (lsl *labelStatusLine) widgets() (walk.Widget, walk.Widget) { } func (lsl *labelStatusLine) update(state service.TunnelState) { - margin := lsl.label.DPI() / 48 //TODO: Do some sort of dynamic DPI calculation here - imageSize := lsl.label.SizeHint() - imageSize.Width = imageSize.Height - //TODO: the *2 in the Y coordinate fixes weird alignment bugs. Why? - imageRect := walk.Rectangle{margin, margin * 2, imageSize.Width - margin*2, imageSize.Height - margin*2} - img := lsl.cachedImages[rectAndSizeAndState{imageRect, imageSize, state}] - if img == nil { - img, _ = walk.NewBitmapWithTransparentPixels(imageSize) - canvas, _ := walk.NewCanvasFromImage(img) - icon, err := iconForState(state, imageRect.Size().Width) - if err == nil { - canvas.DrawImageStretched(icon, imageRect) - canvas.Dispose() - lsl.cachedImages[rectAndSizeAndState{imageRect, imageSize, state}] = img - } else { - canvas.Dispose() - img.Dispose() - img = nil - } + icon, err := iconForState(state, 14) + if err == nil { + lsl.statusImage.SetImage(icon) + } else { + lsl.statusImage.SetImage(nil) } - lsl.statusImage.SetImage(img) + s, e := lsl.statusLabel.TextSelection() lsl.statusLabel.SetText(textForState(state, false)) lsl.statusLabel.SetTextSelection(s, e) @@ -115,8 +100,6 @@ func (lsl *labelStatusLine) update(state service.TunnelState) { func newLabelStatusLine(parent walk.Container) *labelStatusLine { lsl := new(labelStatusLine) - lsl.cachedImages = make(map[rectAndSizeAndState]walk.Image) - lsl.label, _ = walk.NewTextLabel(parent) lsl.label.SetText("Status:") lsl.label.SetTextAlignment(walk.AlignHFarVNear) @@ -129,6 +112,7 @@ func newLabelStatusLine(parent walk.Container) *labelStatusLine { lsl.statusComposite.SetLayout(layout) lsl.statusImage, _ = walk.NewImageView(lsl.statusComposite) + lsl.statusImage.SetMargin(2) lsl.statusImage.SetMode(walk.ImageViewModeIdeal) lsl.statusLabel, _ = walk.NewLineEdit(lsl.statusComposite) diff --git a/ui/editdialog.go b/ui/editdialog.go index d55e5486..f46e91e2 100644 --- a/ui/editdialog.go +++ b/ui/editdialog.go @@ -58,7 +58,7 @@ func runTunnelEditDialog(owner walk.Form, tunnel *service.Tunnel, clone bool) *c dlg.SetTitle(title) dlg.SetLayout(layout) dlg.SetMinMaxSize(walk.Size{500, 400}, walk.Size{0, 0}) - if icon, err := loadSystemIcon("imageres", 109, dlg.DPI()/3); err == nil { //TODO: calculate DPI dynamically + if icon, err := loadSystemIcon("imageres", 109, 32); err == nil { dlg.SetIcon(icon) } diff --git a/ui/iconprovider.go b/ui/iconprovider.go index 52150a46..d7d6fbcf 100644 --- a/ui/iconprovider.go +++ b/ui/iconprovider.go @@ -6,13 +6,7 @@ package ui import ( - "fmt" - "path" - "syscall" - "github.com/lxn/walk" - "github.com/lxn/win" - "golang.org/x/sys/windows" "golang.zx2c4.com/wireguard/windows/service" ) @@ -27,56 +21,44 @@ type widthAndDllIdx struct { dll string } -var cachedOverlayIconsForWidthAndState = make(map[widthAndState]*walk.Icon) +var cachedOverlayIconsForWidthAndState = make(map[widthAndState]walk.Image) -func iconWithOverlayForState(state service.TunnelState, size int) (icon *walk.Icon, err error) { +func iconWithOverlayForState(state service.TunnelState, size int) (icon walk.Image, err error) { icon = cachedOverlayIconsForWidthAndState[widthAndState{size, state}] if icon != nil { return } + wireguardIcon, err := loadLogoIcon(size) if err != nil { return } + if state == service.TunnelStopped { return wireguardIcon, err //TODO: if we find something prettier than the gray dot, then remove this clause } - iconSize := wireguardIcon.Size() - bmp, err := walk.NewBitmapWithTransparentPixels(iconSize) - if err != nil { - return - } - defer bmp.Dispose() - - canvas, err := walk.NewCanvasFromImage(bmp) - if err != nil { - return - } - defer canvas.Dispose() - - err = canvas.DrawImage(wireguardIcon, walk.Point{}) - if err != nil { - return - } + iconSize := wireguardIcon.Size() w := int(float64(iconSize.Width) * 0.65) h := int(float64(iconSize.Height) * 0.65) - bounds := walk.Rectangle{iconSize.Width - w, iconSize.Height - h, w, h} - overlayIcon, err := iconForState(state, bounds.Width) - if err != nil { - return - } - defer overlayIcon.Dispose() - err = canvas.DrawImageStretched(overlayIcon, bounds) + overlayBounds := walk.Rectangle{iconSize.Width - w, iconSize.Height - h, w, h} + overlayIcon, err := iconForState(state, overlayBounds.Width) if err != nil { return } - canvas.Dispose() - icon, err = walk.NewIconFromBitmap(bmp) - if err == nil { - cachedOverlayIconsForWidthAndState[widthAndState{size, state}] = icon - } + icon = walk.NewPaintFuncImage(walk.Size{size, size}, func(canvas *walk.Canvas, bounds walk.Rectangle) error { + if err := canvas.DrawImageStretched(wireguardIcon, bounds); err != nil { + return err + } + if err := canvas.DrawImageStretched(overlayIcon, overlayBounds); err != nil { + return err + } + return nil + }) + + cachedOverlayIconsForWidthAndState[widthAndState{size, state}] = icon + return } @@ -130,16 +112,7 @@ func loadSystemIcon(dll string, index int32, size int) (icon *walk.Icon, err err if icon != nil { return } - system32, err := windows.GetSystemDirectory() - if err != nil { - return - } - var hicon win.HICON - ret := win.SHDefExtractIcon(windows.StringToUTF16Ptr(path.Join(system32, dll+".dll")), index, 0, &hicon, nil, uint32(size)) - if ret != 0 { - return nil, fmt.Errorf("Unable to find icon %d of %s: %v", index, dll, syscall.Errno(ret)) - } - icon, err = walk.NewIconFromHICON(hicon) + icon, err = walk.NewIconFromSysDLLWithSize(dll, int(index), size) if err == nil { cachedSystemIconsForWidthAndDllIdx[widthAndDllIdx{size, index, dll}] = icon } diff --git a/ui/listview.go b/ui/listview.go index 17df1b23..a8c5de89 100644 --- a/ui/listview.go +++ b/ui/listview.go @@ -129,7 +129,7 @@ func (tv *ListView) StyleCell(style *walk.CellStyle) { if err != nil { return } - margin := tv.DPI() / 48 //TODO: Do some sort of dynamic DPI calculation here + const margin = 2 b.X = margin b.Y += margin b.Height -= margin * 2 diff --git a/ui/managewindow.go b/ui/managewindow.go index 8783e635..b1a22ac3 100644 --- a/ui/managewindow.go +++ b/ui/managewindow.go @@ -56,7 +56,7 @@ func NewManageTunnelsWindow() (*ManageTunnelsWindow, error) { win.ChangeWindowMessageFilterEx(mtw.Handle(), raiseMsg, win.MSGFLT_ALLOW, nil) mtw.SetPersistent(true) - if icon, err := loadLogoIcon(mtw.DPI() / 3); err == nil { //TODO: calculate DPI dynamically + if icon, err := loadLogoIcon(32); err == nil { mtw.SetIcon(icon) } mtw.SetTitle("WireGuard") @@ -140,7 +140,7 @@ func (mtw *ManageTunnelsWindow) updateProgressIndicator(globalState service.Tunn default: pi.SetState(walk.PINoProgress) } - if icon, err := iconForState(globalState, mtw.DPI()/6); err == nil { //TODO: calculate DPI dynamically + if icon, err := iconForState(globalState, 16); err == nil { if globalState == service.TunnelStopped { icon = nil } diff --git a/ui/tray.go b/ui/tray.go index 7b5e55ca..d376e749 100644 --- a/ui/tray.go +++ b/ui/tray.go @@ -55,7 +55,7 @@ func (tray *Tray) setup() error { tray.SetToolTip("WireGuard: Deactivated") tray.SetVisible(true) - if icon, err := loadLogoIcon(tray.mtw.DPI() / 6); err == nil { //TODO: calculate DPI dynamically + if icon, err := loadLogoIcon(16); err == nil { tray.SetIcon(icon) } @@ -219,7 +219,7 @@ func (tray *Tray) onTunnelChange(tunnel *service.Tunnel, state service.TunnelSta } func (tray *Tray) updateGlobalState(globalState service.TunnelState) { - if icon, err := iconWithOverlayForState(globalState, tray.mtw.DPI()/6); err == nil { //TODO: calculate DPI dynamically + if icon, err := iconWithOverlayForState(globalState, 16); err == nil { tray.SetIcon(icon) } @@ -287,14 +287,14 @@ func (tray *Tray) SetTunnelState(tunnel *service.Tunnel, state service.TunnelSta tunnelAction.SetEnabled(true) tunnelAction.SetChecked(true) if !wasChecked && showNotifications { - icon, _ := iconWithOverlayForState(state, tray.mtw.DPI()*4/3) //TODO: calculate dpi dynamically + icon, _ := iconWithOverlayForState(state, 128) tray.ShowCustom("WireGuard Activated", fmt.Sprintf("The %s tunnel has been activated.", tunnel.Name), icon) } case service.TunnelStopped: tunnelAction.SetChecked(false) if wasChecked && showNotifications { - icon, _ := loadSystemIcon("imageres", 26, tray.mtw.DPI()*4/3) //TODO: this icon isn't very good..., also calculate dpi dynamically + icon, _ := loadSystemIcon("imageres", 26, 128) //TODO: this icon isn't very good... tray.ShowCustom("WireGuard Deactivated", fmt.Sprintf("The %s tunnel has been deactivated.", tunnel.Name), icon) } } @@ -303,10 +303,8 @@ func (tray *Tray) SetTunnelState(tunnel *service.Tunnel, state service.TunnelSta func (tray *Tray) UpdateFound() { action := walk.NewAction() action.SetText("An Update is Available!") - iconSize := tray.mtw.DPI() / 6 //TODO: This should use dynamic DPI. - menuIcon, _ := loadSystemIcon("imageres", 1, iconSize) - bitmap, _ := walk.NewBitmapFromIcon(menuIcon, walk.Size{iconSize, iconSize}) - action.SetImage(bitmap) + menuIcon, _ := loadSystemIcon("imageres", 1, 16) + action.SetImage(menuIcon) action.SetDefault(true) showUpdateTab := func() { if !tray.mtw.Visible() { @@ -320,7 +318,7 @@ func (tray *Tray) UpdateFound() { tray.ContextMenu().Actions().Insert(tray.ContextMenu().Actions().Len()-2, action) showUpdateBalloon := func() { - icon, _ := loadSystemIcon("imageres", 1, tray.mtw.DPI()*4/3) //TODO: calculate DPI dynamically + icon, _ := loadSystemIcon("imageres", 1, 128) tray.ShowCustom("WireGuard Update Available", "An update to WireGuard is now available. You are advised to update as soon as possible.", icon) } diff --git a/ui/tunnelspage.go b/ui/tunnelspage.go index 192f6420..d6deffff 100644 --- a/ui/tunnelspage.go +++ b/ui/tunnelspage.go @@ -121,33 +121,27 @@ func (tp *TunnelsPage) CreateToolbar() { toolBarContainer.SetLayout(hlayout) tp.listToolbar, _ = walk.NewToolBarWithOrientationAndButtonStyle(toolBarContainer, walk.Horizontal, walk.ToolBarButtonImageBeforeText) - imageSize := walk.Size{tp.DPI() / 6, tp.DPI() / 6} // Dividing by six is the same as dividing by 96 and multiplying by 16. TODO: Use dynamic DPI - imageList, _ := walk.NewImageList(imageSize, walk.RGB(0, 0, 0)) - tp.listToolbar.SetImageList(imageList) addMenu, _ := walk.NewMenu() tp.AddDisposable(addMenu) importAction := walk.NewAction() importAction.SetText("Import tunnel(s) from file...") - importActionIcon, _ := loadSystemIcon("imageres", 3, imageSize.Width) - importActionImage, _ := walk.NewBitmapFromIcon(importActionIcon, imageSize) - importAction.SetImage(importActionImage) + importActionIcon, _ := loadSystemIcon("imageres", 3, 16) + importAction.SetImage(importActionIcon) importAction.SetShortcut(walk.Shortcut{walk.ModControl, walk.KeyO}) importAction.SetDefault(true) importAction.Triggered().Attach(tp.onImport) addMenu.Actions().Add(importAction) addAction := walk.NewAction() addAction.SetText("Add empty tunnel...") - addActionIcon, _ := loadSystemIcon("imageres", 2, imageSize.Width) - addActionImage, _ := walk.NewBitmapFromIcon(addActionIcon, imageSize) - addAction.SetImage(addActionImage) + addActionIcon, _ := loadSystemIcon("imageres", 2, 16) + addAction.SetImage(addActionIcon) addAction.SetShortcut(walk.Shortcut{walk.ModControl, walk.KeyN}) addAction.Triggered().Attach(tp.onAddTunnel) addMenu.Actions().Add(addAction) addMenuAction := walk.NewMenuAction(addMenu) - addMenuActionIcon, _ := loadSystemIcon("shell32", 149, imageSize.Width) - addMenuActionImage, _ := walk.NewBitmapFromIcon(addMenuActionIcon, imageSize) - addMenuAction.SetImage(addMenuActionImage) + addMenuActionIcon, _ := loadSystemIcon("shell32", 149, 16) + addMenuAction.SetImage(addMenuActionIcon) addMenuAction.SetText("Add Tunnel") addMenuAction.SetToolTip(importAction.Text()) addMenuAction.Triggered().Attach(tp.onImport) @@ -156,9 +150,8 @@ func (tp *TunnelsPage) CreateToolbar() { tp.listToolbar.Actions().Add(walk.NewSeparatorAction()) deleteAction := walk.NewAction() - deleteActionIcon, _ := loadSystemIcon("shell32", 131, imageSize.Width) - deleteActionImage, _ := walk.NewBitmapFromIcon(deleteActionIcon, imageSize) - deleteAction.SetImage(deleteActionImage) + deleteActionIcon, _ := loadSystemIcon("shell32", 131, 16) + deleteAction.SetImage(deleteActionIcon) deleteAction.SetShortcut(walk.Shortcut{0, walk.KeyDelete}) deleteAction.SetToolTip("Remove selected tunnel(s)") deleteAction.Triggered().Attach(tp.onDelete) @@ -166,15 +159,18 @@ func (tp *TunnelsPage) CreateToolbar() { tp.listToolbar.Actions().Add(walk.NewSeparatorAction()) exportAction := walk.NewAction() - exportActionIcon, _ := loadSystemIcon("imageres", 165, imageSize.Width) // Or "shell32", 45? - exportActionImage, _ := walk.NewBitmapFromIcon(exportActionIcon, imageSize) - exportAction.SetImage(exportActionImage) + exportActionIcon, _ := loadSystemIcon("imageres", 165, 16) // Or "shell32", 45? + exportAction.SetImage(exportActionIcon) exportAction.SetToolTip("Export all tunnels to zip...") exportAction.Triggered().Attach(tp.onExportTunnels) tp.listToolbar.Actions().Add(exportAction) - toolbarWidth := tp.listToolbar.SizeHint().Width - tp.listContainer.SetMinMaxSizePixels(walk.Size{toolbarWidth, 0}, walk.Size{toolbarWidth, 0}) + fixContainerWidthToToolbarWidth := func() { + toolbarWidth := tp.listToolbar.SizeHint().Width + tp.listContainer.SetMinMaxSizePixels(walk.Size{toolbarWidth, 0}, walk.Size{toolbarWidth, 0}) + } + fixContainerWidthToToolbarWidth() + tp.listToolbar.SizeChanged().Attach(fixContainerWidthToToolbarWidth) contextMenu, _ := walk.NewMenu() toggleAction := walk.NewAction() diff --git a/ui/updatepage.go b/ui/updatepage.go index b50427ce..fcc9a356 100644 --- a/ui/updatepage.go +++ b/ui/updatepage.go @@ -27,10 +27,8 @@ func NewUpdatePage() (*UpdatePage, error) { up.SetTitle("An Update is Available!") - iconSize := up.DPI() / 6 - tabIcon, _ := loadSystemIcon("imageres", 1, iconSize) - bitmap, _ := walk.NewBitmapFromIcon(tabIcon, walk.Size{iconSize, iconSize}) //TODO: this should use dynamic DPI - up.SetImage(bitmap) + tabIcon, _ := loadSystemIcon("imageres", 1, 16) + up.SetImage(tabIcon) up.SetLayout(walk.NewVBoxLayout()) @@ -46,7 +44,7 @@ func NewUpdatePage() (*UpdatePage, error) { bar.SetVisible(false) button, _ := walk.NewPushButton(up) - updateIcon, _ := loadSystemIcon("shell32", 46, bar.HeightPixels()) + updateIcon, _ := loadSystemIcon("shell32", 46, bar.Height()) button.SetImage(updateIcon) button.SetText("Update Now") -- cgit v1.2.3-59-g8ed1b