Skip to content

Commit

Permalink
Merge pull request #2258 from AmanitaVerna/fix2222
Browse files Browse the repository at this point in the history
- Replaces the math used for blending the hover color in button.go's buttonRenderer's buttonColor() to fix #2222.
- Fixes the last remaining cases of #2220 that I've found, in buttonColor() and in button.go's newButtonTapAnimation().
- Adds a math-based test which uses an alternate way of calculating the same blending operation to test buttonRenderer's hover code against (TestButton_Hover_Math in button_internal_test.go).
- Updates the hover test image used by button_test.go's TestButton_Hover()
- Updates a bunch of hover-related tests so that they look for the corrected output from buttonColor(), by updating the four XML files and one image that they test against.

Fixes #2222
Fixes #2220
  • Loading branch information
Jacalz committed May 27, 2021
2 parents 9766815 + 6d5e9c6 commit 6e9d422
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 23 deletions.
2 changes: 1 addition & 1 deletion container/testdata/apptabs/desktop/hover_overflow.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</widget>
</container>
<widget pos="122,0" size="28x29" type="*widget.Button">
<rectangle fillColor="rgba(46,48,48,255)" size="28x29"/>
<rectangle fillColor="rgba(62,78,78,255)" size="28x29"/>
<rectangle size="0x0"/>
<image fillMode="contain" pos="4,4" rsc="more-horizontal.svg" size="iconInlineSize" themed="default"/>
</widget>
Expand Down
2 changes: 1 addition & 1 deletion container/testdata/doctabs/desktop/hover_all_tabs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<image fillMode="contain" pos="4,6" rsc="contentAddIcon" size="iconInlineSize"/>
</widget>
<widget pos="32,0" size="28x32" type="*widget.Button">
<rectangle fillColor="rgba(46,48,48,255)" size="28x32"/>
<rectangle fillColor="rgba(62,78,78,255)" size="28x32"/>
<rectangle size="0x0"/>
<image fillMode="contain" pos="4,6" rsc="more-horizontal.svg" size="iconInlineSize" themed="default"/>
</widget>
Expand Down
2 changes: 1 addition & 1 deletion container/testdata/doctabs/desktop/hover_create_tab.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
</widget>
<container pos="90,0" size="60x32">
<widget size="28x32" type="*widget.Button">
<rectangle fillColor="rgba(46,48,48,255)" size="28x32"/>
<rectangle fillColor="rgba(62,78,78,255)" size="28x32"/>
<rectangle size="0x0"/>
<image fillMode="contain" pos="4,6" rsc="contentAddIcon" size="iconInlineSize"/>
</widget>
Expand Down
2 changes: 1 addition & 1 deletion container/testdata/doctabs/desktop/hover_second.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
</widget>
<container pos="90,0" size="60x32">
<widget size="28x32" type="*widget.Button">
<rectangle fillColor="rgba(46,48,48,255)" size="28x32"/>
<rectangle fillColor="rgba(62,78,78,255)" size="28x32"/>
<rectangle size="0x0"/>
<image fillMode="contain" pos="4,6" rsc="contentAddIcon" size="iconInlineSize"/>
</widget>
Expand Down
Binary file modified internal/driver/glfw/testdata/menu_bar_hovered_content.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 10 additions & 7 deletions widget/button.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fyne.io/fyne/v2"
"fyne.io/fyne/v2/canvas"
"fyne.io/fyne/v2/driver/desktop"
col "fyne.io/fyne/v2/internal/color"
"fyne.io/fyne/v2/internal/widget"
"fyne.io/fyne/v2/layout"
"fyne.io/fyne/v2/theme"
Expand Down Expand Up @@ -314,17 +315,19 @@ func (r *buttonRenderer) buttonColor() color.Color {
bg = theme.PrimaryColor()
}

// This alpha blends with the over operator, and accounts for RGBA() returning alpha-premultiplied values
dstR, dstG, dstB, dstA := bg.RGBA()
srcR, srcG, srcB, srcA := theme.HoverColor().RGBA()

srcAlpha := float32(srcA) / 0xFFFF
dstAlpha := float32(dstA) / 0xFFFF
targetAlpha := 1 - srcAlpha*dstAlpha

outAlpha := srcAlpha + targetAlpha
outR := (srcAlpha*float32(srcR) + targetAlpha*float32(dstR)) / outAlpha
outG := (srcAlpha*float32(srcG) + targetAlpha*float32(dstG)) / outAlpha
outB := (srcAlpha*float32(srcB) + targetAlpha*float32(dstB)) / outAlpha
return color.RGBA{R: uint8(uint32(outR) >> 8), G: uint8(uint32(outG) >> 8), B: uint8(uint32(outB) >> 8), A: uint8(outAlpha * 0xFF)}
outAlpha := srcAlpha + dstAlpha*(1-srcAlpha)
outR := srcR + uint32(float32(dstR)*(1-srcAlpha))
outG := srcG + uint32(float32(dstG)*(1-srcAlpha))
outB := srcB + uint32(float32(dstB)*(1-srcAlpha))
// We create an RGBA64 here because the color components are already alpha-premultiplied 16-bit values (they're just stored in uint32s).
return color.RGBA64{R: uint16(outR), G: uint16(outG), B: uint16(outB), A: uint16(outAlpha * 0xFFFF)}
case r.button.Importance == HighImportance:
return theme.PrimaryColor()
default:
Expand Down Expand Up @@ -386,7 +389,7 @@ func newButtonTapAnimation(bg *canvas.Rectangle, w fyne.Widget) *fyne.Animation
bg.Resize(fyne.NewSize(size*2, w.Size().Height-theme.Padding()))
bg.Move(fyne.NewPos(mid-size, theme.Padding()/2))

r, g, bb, a := theme.PressedColor().RGBA()
r, g, bb, a := col.ToNRGBA(theme.PressedColor())
aa := uint8(a)
fade := aa - uint8(float32(aa)*done)
bg.FillColor = &color.NRGBA{R: uint8(r), G: uint8(g), B: uint8(bb), A: fade}
Expand Down
40 changes: 40 additions & 0 deletions widget/button_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package widget

import (
"fmt"
"image/color"
"testing"

"fyne.io/fyne/v2"
"fyne.io/fyne/v2/internal/cache"
col "fyne.io/fyne/v2/internal/color"
"fyne.io/fyne/v2/internal/widget"
"fyne.io/fyne/v2/test"
"fyne.io/fyne/v2/theme"
Expand All @@ -32,6 +34,44 @@ func TestButton_DisabledColor(t *testing.T) {
assert.Equal(t, bg, theme.DisabledButtonColor())
}

func TestButton_Hover_Math(t *testing.T) {
button := NewButtonWithIcon("Test", theme.HomeIcon(), func() {})
render := test.WidgetRenderer(button).(*buttonRenderer)
button.hovered = true
// unpremultiplied over operator:
// outA = srcA + dstA*(1-srcA)
// outC = (srcC*srcA + dstC*dstA*(1-srcA))/outA

// premultiplied over operator:
// outC = srcC + dstC*(1-srcA)
// outA = srcA + dstA*(1-srcA)

// the buttonRenderer's buttonColor() currently calls RGBA(), turns the alpha into a 0=<a<=1 float32,
// and blends with the premultiplied over operator, storing the result in an RGBA64.

// We're going to call ToNRGBA instead (which returns 8-bit components), use the unpremultiplied over operator,
// store the result in an NRGBA, then convert what buttonColor() returns to an nrgba and compare.
bg := theme.ButtonColor()
if render.button.Importance == HighImportance {
bg = theme.PrimaryColor()
}
dstR, dstG, dstB, dstA := col.ToNRGBA(bg)
srcR, srcG, srcB, srcA := col.ToNRGBA(theme.HoverColor())

srcAlpha := float32(srcA) / 0xFF
dstAlpha := float32(dstA) / 0xFF

outAlpha := (srcAlpha + dstAlpha*(1-srcAlpha))
outR := uint32((float32(srcR)*srcAlpha + float32(dstR)*dstAlpha*(1-srcAlpha)) / outAlpha)
outG := uint32((float32(srcG)*srcAlpha + float32(dstG)*dstAlpha*(1-srcAlpha)) / outAlpha)
outB := uint32((float32(srcB)*srcAlpha + float32(dstB)*dstAlpha*(1-srcAlpha)) / outAlpha)

nrgba := color.NRGBA{R: uint8(outR), G: uint8(outG), B: uint8(outB), A: uint8(outAlpha * 0xFF)}
bcn := color.NRGBAModel.Convert(render.buttonColor())

assert.Equal(t, nrgba, bcn)
}

func TestButton_DisabledIcon(t *testing.T) {
button := NewButtonWithIcon("Test", theme.CancelIcon(), nil)
render := test.WidgetRenderer(button).(*buttonRenderer)
Expand Down
8 changes: 4 additions & 4 deletions widget/button_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ func TestButton_Tapped(t *testing.T) {
}

func TestButton_Disable(t *testing.T) {
app := test.NewApp()
test.NewApp()
defer test.NewApp()
app.Settings().SetTheme(theme.LightTheme())
test.ApplyTheme(t, theme.LightTheme())

tapped := false
button := widget.NewButtonWithIcon("Test", theme.HomeIcon(), func() {
Expand Down Expand Up @@ -122,9 +122,9 @@ func TestButton_Disabled(t *testing.T) {
}

func TestButton_Hover(t *testing.T) {
app := test.NewApp()
test.NewApp()
defer test.NewApp()
app.Settings().SetTheme(theme.LightTheme())
test.ApplyTheme(t, theme.LightTheme())

b := widget.NewButtonWithIcon("Test", theme.HomeIcon(), func() {})
w := test.NewWindow(b)
Expand Down
3 changes: 1 addition & 2 deletions widget/entry_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"testing"

"fyne.io/fyne/v2"
"fyne.io/fyne/v2/data/validation"
"fyne.io/fyne/v2/test"
"fyne.io/fyne/v2/theme"
Expand Down Expand Up @@ -89,7 +88,7 @@ func TestEntry_NotEmptyValidator(t *testing.T) {

func TestEntry_SetValidationError(t *testing.T) {
entry, window := setupImageTest(t, false)
fyne.CurrentApp().Settings().SetTheme(theme.LightTheme())
test.ApplyTheme(t, theme.LightTheme())
defer teardownImageTest(window)
c := window.Canvas()

Expand Down
8 changes: 4 additions & 4 deletions widget/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ func TestTable_ChangeTheme(t *testing.T) {
}

func TestTable_Filled(t *testing.T) {
app := test.NewApp()
test.NewApp()
defer test.NewApp()
app.Settings().SetTheme(theme.LightTheme())
test.ApplyTheme(t, theme.LightTheme())

table := NewTable(
func() (int, int) { return 5, 5 },
Expand Down Expand Up @@ -261,9 +261,9 @@ func TestTable_Select(t *testing.T) {
}

func TestTable_SetColumnWidth(t *testing.T) {
app := test.NewApp()
test.NewApp()
defer test.NewApp()
app.Settings().SetTheme(theme.LightTheme())
test.ApplyTheme(t, theme.LightTheme())

table := NewTable(
func() (int, int) { return 5, 5 },
Expand Down
Binary file modified widget/testdata/button/hovered.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions widget/tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,9 @@ func TestTree_Move(t *testing.T) {
}

func TestTree_Refresh(t *testing.T) {
app := test.NewApp()
test.NewApp()
defer test.NewApp()
app.Settings().SetTheme(theme.LightTheme())
test.ApplyTheme(t, theme.LightTheme())

value := "Foo Leaf"
tree := widget.NewTreeWithStrings(treeData)
Expand Down

0 comments on commit 6e9d422

Please sign in to comment.