Merge branch 'fix/hint-review-adapters' into feat/hint-hybrid
21 files · +1123 -87
M .mockery.yml +5 -0
| @@ -125,6 +125,11 @@ packages: | |||
| 125 | 125 | dir: internal/adapters/atspi | |
| 126 | 126 | pkgname: atspi | |
| 127 | 127 | filename: transport_mock_test.go | |
| 128 | + | connector: | |
| 129 | + | config: | |
| 130 | + | dir: internal/adapters/atspi | |
| 131 | + | pkgname: atspi | |
| 132 | + | filename: connector_mock_test.go | |
| 128 | 133 | github.com/bnema/neferwl/internal/adapters/screencast: | |
| 129 | 134 | interfaces: | |
| 130 | 135 | bus: | |
| @@ -125,6 +125,11 @@ packages: | |||||
| 125 | dir: internal/adapters/atspi | 125 | dir: internal/adapters/atspi | ||
| 126 | pkgname: atspi | 126 | pkgname: atspi | ||
| 127 | filename: transport_mock_test.go | 127 | filename: transport_mock_test.go | ||
| 128 | + | connector: | |||
| 129 | + | config: | |||
| 130 | + | dir: internal/adapters/atspi | |||
| 131 | + | pkgname: atspi | |||
| 132 | + | filename: connector_mock_test.go | |||
| 128 | github.com/bnema/neferwl/internal/adapters/screencast: | 133 | github.com/bnema/neferwl/internal/adapters/screencast: | ||
| 129 | interfaces: | 134 | interfaces: | ||
| 130 | bus: | 135 | bus: | ||
M docs/config.md +6 -4
| @@ -324,12 +324,14 @@ a link; accessibility targets get a plain click. | |||
| 324 | 324 | | Source | Apps | Accuracy | Speed | | |
| 325 | 325 | |---|---|---|---| | |
| 326 | 326 | | Accessibility tree (AT-SPI) | Firefox, Chromium, GTK, Qt, Electron | Exact: the app reports its own links and buttons | A few milliseconds | | |
| 327 | - | | Text recognition (OCR) | Everything else: terminals, apps without accessibility | Best effort: only URLs and domains written on screen | 150 ms or more, more on large screens | | |
| 327 | + | | Text recognition (OCR) | Everything else: terminals, apps without accessibility | Best effort: URLs, domains and underlined text written on screen | 150 ms or more, more on large screens | | |
| 328 | 328 | ||
| 329 | 329 | NeferWL asks the accessibility tree first. Windows it does not cover are read | |
| 330 | - | with OCR, which finds visible URLs, domains and email addresses. OCR cannot | |
| 331 | - | see a link whose address is hidden behind its text, and it can misread small | |
| 332 | - | or low-contrast text. | |
| 330 | + | with OCR, which finds visible URLs, domains, email addresses and underlined | |
| 331 | + | text. Underlined words get a plain click, and the words of one continuous | |
| 332 | + | underline form one target; URLs keep the `hint.text-click` modifiers. OCR | |
| 333 | + | cannot see a link whose address is hidden behind plain text, and it can misread | |
| 334 | + | small or low-contrast text. | |
| 333 | 335 | ||
| 334 | 336 | ### Requirements | |
| 335 | 337 | ||
| @@ -324,12 +324,14 @@ a link; accessibility targets get a plain click. | |||||
| 324 | | Source | Apps | Accuracy | Speed | | 324 | | Source | Apps | Accuracy | Speed | | ||
| 325 | |---|---|---|---| | 325 | |---|---|---|---| | ||
| 326 | | Accessibility tree (AT-SPI) | Firefox, Chromium, GTK, Qt, Electron | Exact: the app reports its own links and buttons | A few milliseconds | | 326 | | Accessibility tree (AT-SPI) | Firefox, Chromium, GTK, Qt, Electron | Exact: the app reports its own links and buttons | A few milliseconds | | ||
| 327 | - | | Text recognition (OCR) | Everything else: terminals, apps without accessibility | Best effort: only URLs and domains written on screen | 150 ms or more, more on large screens | | 327 | + | | Text recognition (OCR) | Everything else: terminals, apps without accessibility | Best effort: URLs, domains and underlined text written on screen | 150 ms or more, more on large screens | |
| 328 | 328 | ||||
| 329 | NeferWL asks the accessibility tree first. Windows it does not cover are read | 329 | NeferWL asks the accessibility tree first. Windows it does not cover are read | ||
| 330 | - | with OCR, which finds visible URLs, domains and email addresses. OCR cannot | 330 | + | with OCR, which finds visible URLs, domains, email addresses and underlined |
| 331 | - | see a link whose address is hidden behind its text, and it can misread small | 331 | + | text. Underlined words get a plain click, and the words of one continuous |
| 332 | - | or low-contrast text. | 332 | + | underline form one target; URLs keep the `hint.text-click` modifiers. OCR |
| 333 | + | cannot see a link whose address is hidden behind plain text, and it can misread | |||
| 334 | + | small or low-contrast text. | |||
| 333 | 335 | ||||
| 334 | ### Requirements | 336 | ### Requirements | ||
| 335 | 337 | ||||
M internal/adapters/atspi/bus_test.go +31 -0
| @@ -164,3 +164,34 @@ func TestDoTimesOutOnSilentApp(t *testing.T) { | |||
| 164 | 164 | require.ErrorIs(t, rep[0].err, ErrTimeout) | |
| 165 | 165 | require.Less(t, time.Since(t0), 500*time.Millisecond) | |
| 166 | 166 | } | |
| 167 | + | ||
| 168 | + | // A call left unanswered for idle gets ErrTimeout without breaking the | |
| 169 | + | // connection; a batch that hits its deadline breaks it, and Do returns near | |
| 170 | + | // the deadline even though the idle timeout is longer. | |
| 171 | + | func TestDoDeadlineBreaksConnection(t *testing.T) { | |
| 172 | + | addr := startBus(t) | |
| 173 | + | app, err := zerobus.Dial(addr) | |
| 174 | + | require.NoError(t, err) | |
| 175 | + | t.Cleanup(func() { _ = app.Close() }) | |
| 176 | + | _, err = app.RequestName("org.test.Mute", 0) | |
| 177 | + | require.NoError(t, err) | |
| 178 | + | ||
| 179 | + | cc, err := zerobus.Dial(addr) | |
| 180 | + | require.NoError(t, err) | |
| 181 | + | b, err := newConn(cc) | |
| 182 | + | require.NoError(t, err) | |
| 183 | + | t.Cleanup(func() { _ = b.Close() }) | |
| 184 | + | ||
| 185 | + | mute := []call{{op: opRole, name: "org.test.Mute", path: "/x"}} | |
| 186 | + | rep := b.Do(time.Now().Add(time.Second), 40*time.Millisecond, mute) | |
| 187 | + | require.ErrorIs(t, rep[0].err, ErrTimeout) | |
| 188 | + | require.NotErrorIs(t, rep[0].err, ErrBroken) | |
| 189 | + | ||
| 190 | + | t0 := time.Now() | |
| 191 | + | rep = b.Do(time.Now().Add(60*time.Millisecond), time.Second, mute) | |
| 192 | + | require.ErrorIs(t, rep[0].err, ErrBroken) | |
| 193 | + | require.Less(t, time.Since(t0), 300*time.Millisecond) | |
| 194 | + | // The socket is closed: the next batch fails at once. | |
| 195 | + | rep = b.Do(time.Now().Add(time.Second), time.Second, mute) | |
| 196 | + | require.ErrorIs(t, rep[0].err, ErrBroken) | |
| 197 | + | } | |
| @@ -164,3 +164,34 @@ func TestDoTimesOutOnSilentApp(t *testing.T) { | |||||
| 164 | require.ErrorIs(t, rep[0].err, ErrTimeout) | 164 | require.ErrorIs(t, rep[0].err, ErrTimeout) | ||
| 165 | require.Less(t, time.Since(t0), 500*time.Millisecond) | 165 | require.Less(t, time.Since(t0), 500*time.Millisecond) | ||
| 166 | } | 166 | } | ||
| 167 | + | ||||
| 168 | + | // A call left unanswered for idle gets ErrTimeout without breaking the | |||
| 169 | + | // connection; a batch that hits its deadline breaks it, and Do returns near | |||
| 170 | + | // the deadline even though the idle timeout is longer. | |||
| 171 | + | func TestDoDeadlineBreaksConnection(t *testing.T) { | |||
| 172 | + | addr := startBus(t) | |||
| 173 | + | app, err := zerobus.Dial(addr) | |||
| 174 | + | require.NoError(t, err) | |||
| 175 | + | t.Cleanup(func() { _ = app.Close() }) | |||
| 176 | + | _, err = app.RequestName("org.test.Mute", 0) | |||
| 177 | + | require.NoError(t, err) | |||
| 178 | + | ||||
| 179 | + | cc, err := zerobus.Dial(addr) | |||
| 180 | + | require.NoError(t, err) | |||
| 181 | + | b, err := newConn(cc) | |||
| 182 | + | require.NoError(t, err) | |||
| 183 | + | t.Cleanup(func() { _ = b.Close() }) | |||
| 184 | + | ||||
| 185 | + | mute := []call{{op: opRole, name: "org.test.Mute", path: "/x"}} | |||
| 186 | + | rep := b.Do(time.Now().Add(time.Second), 40*time.Millisecond, mute) | |||
| 187 | + | require.ErrorIs(t, rep[0].err, ErrTimeout) | |||
| 188 | + | require.NotErrorIs(t, rep[0].err, ErrBroken) | |||
| 189 | + | ||||
| 190 | + | t0 := time.Now() | |||
| 191 | + | rep = b.Do(time.Now().Add(60*time.Millisecond), time.Second, mute) | |||
| 192 | + | require.ErrorIs(t, rep[0].err, ErrBroken) | |||
| 193 | + | require.Less(t, time.Since(t0), 300*time.Millisecond) | |||
| 194 | + | // The socket is closed: the next batch fails at once. | |||
| 195 | + | rep = b.Do(time.Now().Add(time.Second), time.Second, mute) | |||
| 196 | + | require.ErrorIs(t, rep[0].err, ErrBroken) | |||
| 197 | + | } | |||
A internal/adapters/atspi/connector_mock_test.go +109 -0
| @@ -0,0 +1,109 @@ | |||
| 1 | + | // Code generated by mockery; DO NOT EDIT. | |
| 2 | + | // github.com/vektra/mockery | |
| 3 | + | // template: testify | |
| 4 | + | ||
| 5 | + | package atspi | |
| 6 | + | ||
| 7 | + | import ( | |
| 8 | + | "context" | |
| 9 | + | ||
| 10 | + | mock "github.com/stretchr/testify/mock" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // newMockconnector creates a new instance of mockconnector. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |
| 14 | + | // The first argument is typically a *testing.T value. | |
| 15 | + | func newMockconnector(t interface { | |
| 16 | + | mock.TestingT | |
| 17 | + | Cleanup(func()) | |
| 18 | + | }) *mockconnector { | |
| 19 | + | if helper, ok := t.(interface{ Helper() }); ok { | |
| 20 | + | helper.Helper() | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | mock := &mockconnector{} | |
| 24 | + | mock.Mock.Test(t) | |
| 25 | + | ||
| 26 | + | t.Cleanup(func() { | |
| 27 | + | if helper, ok := t.(interface{ Helper() }); ok { | |
| 28 | + | helper.Helper() | |
| 29 | + | } | |
| 30 | + | mock.AssertExpectations(t) | |
| 31 | + | }) | |
| 32 | + | ||
| 33 | + | return mock | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | // mockconnector is an autogenerated mock type for the connector type | |
| 37 | + | type mockconnector struct { | |
| 38 | + | mock.Mock | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | type mockconnector_Expecter struct { | |
| 42 | + | mock *mock.Mock | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | func (_m *mockconnector) EXPECT() *mockconnector_Expecter { | |
| 46 | + | return &mockconnector_Expecter{mock: &_m.Mock} | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | // Connect provides a mock function for the type mockconnector | |
| 50 | + | func (_mock *mockconnector) Connect(ctx context.Context) (transport, error) { | |
| 51 | + | ret := _mock.Called(ctx) | |
| 52 | + | ||
| 53 | + | if len(ret) == 0 { | |
| 54 | + | panic("no return value specified for Connect") | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | var r0 transport | |
| 58 | + | var r1 error | |
| 59 | + | if returnFunc, ok := ret.Get(0).(func(context.Context) (transport, error)); ok { | |
| 60 | + | return returnFunc(ctx) | |
| 61 | + | } | |
| 62 | + | if returnFunc, ok := ret.Get(0).(func(context.Context) transport); ok { | |
| 63 | + | r0 = returnFunc(ctx) | |
| 64 | + | } else { | |
| 65 | + | if ret.Get(0) != nil { | |
| 66 | + | r0 = ret.Get(0).(transport) | |
| 67 | + | } | |
| 68 | + | } | |
| 69 | + | if returnFunc, ok := ret.Get(1).(func(context.Context) error); ok { | |
| 70 | + | r1 = returnFunc(ctx) | |
| 71 | + | } else { | |
| 72 | + | r1 = ret.Error(1) | |
| 73 | + | } | |
| 74 | + | return r0, r1 | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | // mockconnector_Connect_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Connect' | |
| 78 | + | type mockconnector_Connect_Call struct { | |
| 79 | + | *mock.Call | |
| 80 | + | } | |
| 81 | + | ||
| 82 | + | // Connect is a helper method to define mock.On call | |
| 83 | + | // - ctx context.Context | |
| 84 | + | func (_e *mockconnector_Expecter) Connect(ctx any) *mockconnector_Connect_Call { | |
| 85 | + | return &mockconnector_Connect_Call{Call: _e.mock.On("Connect", ctx)} | |
| 86 | + | } | |
| 87 | + | ||
| 88 | + | func (_c *mockconnector_Connect_Call) Run(run func(ctx context.Context)) *mockconnector_Connect_Call { | |
| 89 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 90 | + | var arg0 context.Context | |
| 91 | + | if args[0] != nil { | |
| 92 | + | arg0 = args[0].(context.Context) | |
| 93 | + | } | |
| 94 | + | run( | |
| 95 | + | arg0, | |
| 96 | + | ) | |
| 97 | + | }) | |
| 98 | + | return _c | |
| 99 | + | } | |
| 100 | + | ||
| 101 | + | func (_c *mockconnector_Connect_Call) Return(transportMoqParam transport, err error) *mockconnector_Connect_Call { | |
| 102 | + | _c.Call.Return(transportMoqParam, err) | |
| 103 | + | return _c | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | func (_c *mockconnector_Connect_Call) RunAndReturn(run func(ctx context.Context) (transport, error)) *mockconnector_Connect_Call { | |
| 107 | + | _c.Call.Return(run) | |
| 108 | + | return _c | |
| 109 | + | } | |
| @@ -0,0 +1,109 @@ | |||||
| 1 | + | // Code generated by mockery; DO NOT EDIT. | |||
| 2 | + | // github.com/vektra/mockery | |||
| 3 | + | // template: testify | |||
| 4 | + | ||||
| 5 | + | package atspi | |||
| 6 | + | ||||
| 7 | + | import ( | |||
| 8 | + | "context" | |||
| 9 | + | ||||
| 10 | + | mock "github.com/stretchr/testify/mock" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // newMockconnector creates a new instance of mockconnector. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |||
| 14 | + | // The first argument is typically a *testing.T value. | |||
| 15 | + | func newMockconnector(t interface { | |||
| 16 | + | mock.TestingT | |||
| 17 | + | Cleanup(func()) | |||
| 18 | + | }) *mockconnector { | |||
| 19 | + | if helper, ok := t.(interface{ Helper() }); ok { | |||
| 20 | + | helper.Helper() | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | mock := &mockconnector{} | |||
| 24 | + | mock.Mock.Test(t) | |||
| 25 | + | ||||
| 26 | + | t.Cleanup(func() { | |||
| 27 | + | if helper, ok := t.(interface{ Helper() }); ok { | |||
| 28 | + | helper.Helper() | |||
| 29 | + | } | |||
| 30 | + | mock.AssertExpectations(t) | |||
| 31 | + | }) | |||
| 32 | + | ||||
| 33 | + | return mock | |||
| 34 | + | } | |||
| 35 | + | ||||
| 36 | + | // mockconnector is an autogenerated mock type for the connector type | |||
| 37 | + | type mockconnector struct { | |||
| 38 | + | mock.Mock | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | type mockconnector_Expecter struct { | |||
| 42 | + | mock *mock.Mock | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | func (_m *mockconnector) EXPECT() *mockconnector_Expecter { | |||
| 46 | + | return &mockconnector_Expecter{mock: &_m.Mock} | |||
| 47 | + | } | |||
| 48 | + | ||||
| 49 | + | // Connect provides a mock function for the type mockconnector | |||
| 50 | + | func (_mock *mockconnector) Connect(ctx context.Context) (transport, error) { | |||
| 51 | + | ret := _mock.Called(ctx) | |||
| 52 | + | ||||
| 53 | + | if len(ret) == 0 { | |||
| 54 | + | panic("no return value specified for Connect") | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | var r0 transport | |||
| 58 | + | var r1 error | |||
| 59 | + | if returnFunc, ok := ret.Get(0).(func(context.Context) (transport, error)); ok { | |||
| 60 | + | return returnFunc(ctx) | |||
| 61 | + | } | |||
| 62 | + | if returnFunc, ok := ret.Get(0).(func(context.Context) transport); ok { | |||
| 63 | + | r0 = returnFunc(ctx) | |||
| 64 | + | } else { | |||
| 65 | + | if ret.Get(0) != nil { | |||
| 66 | + | r0 = ret.Get(0).(transport) | |||
| 67 | + | } | |||
| 68 | + | } | |||
| 69 | + | if returnFunc, ok := ret.Get(1).(func(context.Context) error); ok { | |||
| 70 | + | r1 = returnFunc(ctx) | |||
| 71 | + | } else { | |||
| 72 | + | r1 = ret.Error(1) | |||
| 73 | + | } | |||
| 74 | + | return r0, r1 | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | // mockconnector_Connect_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Connect' | |||
| 78 | + | type mockconnector_Connect_Call struct { | |||
| 79 | + | *mock.Call | |||
| 80 | + | } | |||
| 81 | + | ||||
| 82 | + | // Connect is a helper method to define mock.On call | |||
| 83 | + | // - ctx context.Context | |||
| 84 | + | func (_e *mockconnector_Expecter) Connect(ctx any) *mockconnector_Connect_Call { | |||
| 85 | + | return &mockconnector_Connect_Call{Call: _e.mock.On("Connect", ctx)} | |||
| 86 | + | } | |||
| 87 | + | ||||
| 88 | + | func (_c *mockconnector_Connect_Call) Run(run func(ctx context.Context)) *mockconnector_Connect_Call { | |||
| 89 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 90 | + | var arg0 context.Context | |||
| 91 | + | if args[0] != nil { | |||
| 92 | + | arg0 = args[0].(context.Context) | |||
| 93 | + | } | |||
| 94 | + | run( | |||
| 95 | + | arg0, | |||
| 96 | + | ) | |||
| 97 | + | }) | |||
| 98 | + | return _c | |||
| 99 | + | } | |||
| 100 | + | ||||
| 101 | + | func (_c *mockconnector_Connect_Call) Return(transportMoqParam transport, err error) *mockconnector_Connect_Call { | |||
| 102 | + | _c.Call.Return(transportMoqParam, err) | |||
| 103 | + | return _c | |||
| 104 | + | } | |||
| 105 | + | ||||
| 106 | + | func (_c *mockconnector_Connect_Call) RunAndReturn(run func(ctx context.Context) (transport, error)) *mockconnector_Connect_Call { | |||
| 107 | + | _c.Call.Return(run) | |||
| 108 | + | return _c | |||
| 109 | + | } | |||
M internal/adapters/atspi/model.go +35 -21
| @@ -113,12 +113,13 @@ type frameInfo struct { | |||
| 113 | 113 | w, h int // size of the frame, from its window extents | |
| 114 | 114 | } | |
| 115 | 115 | ||
| 116 | - | // matchFrames pairs the windows of one app with its frames. A window is | |
| 117 | - | // paired with a frame when the pairing is certain: | |
| 116 | + | // matchFrames pairs the windows of one app with its frames, one to one. A | |
| 117 | + | // window is paired with a frame when the pairing is certain: | |
| 118 | 118 | // - one window and one frame; | |
| 119 | - | // - the focused window with the one ACTIVE frame; | |
| 120 | - | // - otherwise the one frame whose size is within sizeTolerance of the | |
| 121 | - | // window, provided no other window claims it as well. | |
| 119 | + | // - the focused window with the one ACTIVE frame; that frame is then | |
| 120 | + | // reserved and no other window can take it; | |
| 121 | + | // - otherwise the one unreserved frame whose size is within sizeTolerance | |
| 122 | + | // of the window, provided no other unmatched window claims it as well. | |
| 122 | 123 | // | |
| 123 | 124 | // Ambiguous windows are left out. The result maps the index of a window in | |
| 124 | 125 | // wins to the index of its frame in frames. | |
| @@ -128,31 +129,44 @@ func matchFrames(wins []ports.HintWindow, frames []frameInfo) map[int]int { | |||
| 128 | 129 | out[0] = 0 | |
| 129 | 130 | return out | |
| 130 | 131 | } | |
| 132 | + | // Active matches first: they are reserved against the size matches. | |
| 133 | + | active, n := -1, 0 | |
| 134 | + | for j, f := range frames { | |
| 135 | + | if f.active { | |
| 136 | + | active, n = j, n+1 | |
| 137 | + | } | |
| 138 | + | } | |
| 139 | + | reserved := make([]bool, len(frames)) | |
| 140 | + | done := make([]bool, len(wins)) | |
| 141 | + | if n == 1 { | |
| 142 | + | focused := -1 | |
| 143 | + | count := 0 | |
| 144 | + | for i, w := range wins { | |
| 145 | + | if w.Focused { | |
| 146 | + | focused, count = i, count+1 | |
| 147 | + | } | |
| 148 | + | } | |
| 149 | + | if count == 1 { | |
| 150 | + | out[focused] = active | |
| 151 | + | reserved[active] = true | |
| 152 | + | done[focused] = true | |
| 153 | + | } | |
| 154 | + | } | |
| 131 | 155 | claims := make([][]int, len(wins)) // size candidates per window | |
| 132 | 156 | owners := make([]int, len(frames)) // windows that size-match a frame | |
| 133 | 157 | for i, w := range wins { | |
| 158 | + | if done[i] { | |
| 159 | + | continue | |
| 160 | + | } | |
| 134 | 161 | for j, f := range frames { | |
| 135 | - | if absInt(f.w-w.Rect.W) <= sizeTolerance && absInt(f.h-w.Rect.H) <= sizeTolerance { | |
| 162 | + | if !reserved[j] && absInt(f.w-w.Rect.W) <= sizeTolerance && absInt(f.h-w.Rect.H) <= sizeTolerance { | |
| 136 | 163 | claims[i] = append(claims[i], j) | |
| 137 | 164 | owners[j]++ | |
| 138 | 165 | } | |
| 139 | 166 | } | |
| 140 | 167 | } | |
| 141 | - | for i, w := range wins { | |
| 142 | - | if w.Focused { | |
| 143 | - | active := -1 | |
| 144 | - | n := 0 | |
| 145 | - | for j, f := range frames { | |
| 146 | - | if f.active { | |
| 147 | - | active, n = j, n+1 | |
| 148 | - | } | |
| 149 | - | } | |
| 150 | - | if n == 1 { | |
| 151 | - | out[i] = active | |
| 152 | - | continue | |
| 153 | - | } | |
| 154 | - | } | |
| 155 | - | if len(claims[i]) == 1 && owners[claims[i][0]] == 1 { | |
| 168 | + | for i := range wins { | |
| 169 | + | if !done[i] && len(claims[i]) == 1 && owners[claims[i][0]] == 1 { | |
| 156 | 170 | out[i] = claims[i][0] | |
| 157 | 171 | } | |
| 158 | 172 | } | |
| @@ -113,12 +113,13 @@ type frameInfo struct { | |||||
| 113 | w, h int // size of the frame, from its window extents | 113 | w, h int // size of the frame, from its window extents | ||
| 114 | } | 114 | } | ||
| 115 | 115 | ||||
| 116 | - | // matchFrames pairs the windows of one app with its frames. A window is | 116 | + | // matchFrames pairs the windows of one app with its frames, one to one. A |
| 117 | - | // paired with a frame when the pairing is certain: | 117 | + | // window is paired with a frame when the pairing is certain: |
| 118 | // - one window and one frame; | 118 | // - one window and one frame; | ||
| 119 | - | // - the focused window with the one ACTIVE frame; | 119 | + | // - the focused window with the one ACTIVE frame; that frame is then |
| 120 | - | // - otherwise the one frame whose size is within sizeTolerance of the | 120 | + | // reserved and no other window can take it; |
| 121 | - | // window, provided no other window claims it as well. | 121 | + | // - otherwise the one unreserved frame whose size is within sizeTolerance |
| 122 | + | // of the window, provided no other unmatched window claims it as well. | |||
| 122 | // | 123 | // | ||
| 123 | // Ambiguous windows are left out. The result maps the index of a window in | 124 | // Ambiguous windows are left out. The result maps the index of a window in | ||
| 124 | // wins to the index of its frame in frames. | 125 | // wins to the index of its frame in frames. | ||
| @@ -128,31 +129,44 @@ func matchFrames(wins []ports.HintWindow, frames []frameInfo) map[int]int { | |||||
| 128 | out[0] = 0 | 129 | out[0] = 0 | ||
| 129 | return out | 130 | return out | ||
| 130 | } | 131 | } | ||
| 132 | + | // Active matches first: they are reserved against the size matches. | |||
| 133 | + | active, n := -1, 0 | |||
| 134 | + | for j, f := range frames { | |||
| 135 | + | if f.active { | |||
| 136 | + | active, n = j, n+1 | |||
| 137 | + | } | |||
| 138 | + | } | |||
| 139 | + | reserved := make([]bool, len(frames)) | |||
| 140 | + | done := make([]bool, len(wins)) | |||
| 141 | + | if n == 1 { | |||
| 142 | + | focused := -1 | |||
| 143 | + | count := 0 | |||
| 144 | + | for i, w := range wins { | |||
| 145 | + | if w.Focused { | |||
| 146 | + | focused, count = i, count+1 | |||
| 147 | + | } | |||
| 148 | + | } | |||
| 149 | + | if count == 1 { | |||
| 150 | + | out[focused] = active | |||
| 151 | + | reserved[active] = true | |||
| 152 | + | done[focused] = true | |||
| 153 | + | } | |||
| 154 | + | } | |||
| 131 | claims := make([][]int, len(wins)) // size candidates per window | 155 | claims := make([][]int, len(wins)) // size candidates per window | ||
| 132 | owners := make([]int, len(frames)) // windows that size-match a frame | 156 | owners := make([]int, len(frames)) // windows that size-match a frame | ||
| 133 | for i, w := range wins { | 157 | for i, w := range wins { | ||
| 158 | + | if done[i] { | |||
| 159 | + | continue | |||
| 160 | + | } | |||
| 134 | for j, f := range frames { | 161 | for j, f := range frames { | ||
| 135 | - | if absInt(f.w-w.Rect.W) <= sizeTolerance && absInt(f.h-w.Rect.H) <= sizeTolerance { | 162 | + | if !reserved[j] && absInt(f.w-w.Rect.W) <= sizeTolerance && absInt(f.h-w.Rect.H) <= sizeTolerance { |
| 136 | claims[i] = append(claims[i], j) | 163 | claims[i] = append(claims[i], j) | ||
| 137 | owners[j]++ | 164 | owners[j]++ | ||
| 138 | } | 165 | } | ||
| 139 | } | 166 | } | ||
| 140 | } | 167 | } | ||
| 141 | - | for i, w := range wins { | 168 | + | for i := range wins { |
| 142 | - | if w.Focused { | 169 | + | if !done[i] && len(claims[i]) == 1 && owners[claims[i][0]] == 1 { |
| 143 | - | active := -1 | |||
| 144 | - | n := 0 | |||
| 145 | - | for j, f := range frames { | |||
| 146 | - | if f.active { | |||
| 147 | - | active, n = j, n+1 | |||
| 148 | - | } | |||
| 149 | - | } | |||
| 150 | - | if n == 1 { | |||
| 151 | - | out[i] = active | |||
| 152 | - | continue | |||
| 153 | - | } | |||
| 154 | - | } | |||
| 155 | - | if len(claims[i]) == 1 && owners[claims[i][0]] == 1 { | |||
| 156 | out[i] = claims[i][0] | 170 | out[i] = claims[i][0] | ||
| 157 | } | 171 | } | ||
| 158 | } | 172 | } | ||
M internal/adapters/atspi/model_test.go +16 -3
| @@ -73,9 +73,9 @@ func TestMatchFramesSingle(t *testing.T) { | |||
| 73 | 73 | func TestMatchFramesActive(t *testing.T) { | |
| 74 | 74 | frames := []frameInfo{{w: 800, h: 600}, {w: 800, h: 600, active: true}} | |
| 75 | 75 | got := matchFrames(wins(win(1, 800, 600, true), win(2, 800, 600, false)), frames) | |
| 76 | - | // The focused window takes the ACTIVE frame; the other cannot be told | |
| 77 | - | // apart by size (both frames match it) and is left out. | |
| 78 | - | require.Equal(t, map[int]int{0: 1}, got) | |
| 76 | + | // The focused window takes the ACTIVE frame and reserves it; the other | |
| 77 | + | // window then only has the remaining frame to match. | |
| 78 | + | require.Equal(t, map[int]int{0: 1, 1: 0}, got) | |
| 79 | 79 | } | |
| 80 | 80 | ||
| 81 | 81 | func TestMatchFramesSize(t *testing.T) { | |
| @@ -102,3 +102,16 @@ func TestMatchFramesAmbiguous(t *testing.T) { | |||
| 102 | 102 | // No frame close enough. | |
| 103 | 103 | require.Empty(t, matchFrames(wins(win(1, 200, 200, false), win(2, 300, 300, false)), frames)) | |
| 104 | 104 | } | |
| 105 | + | ||
| 106 | + | // The active frame is reserved by the focused window: an unfocused window of | |
| 107 | + | // the same size as that frame must not be paired with it as well. | |
| 108 | + | func TestMatchFramesOneToOne(t *testing.T) { | |
| 109 | + | frames := []frameInfo{{ref: ref{"a", "/active"}, active: true, w: 400, h: 300}, {ref: ref{"a", "/idle"}, w: 800, h: 600}} | |
| 110 | + | got := matchFrames(wins(win(1, 800, 600, true), win(2, 400, 300, false)), frames) | |
| 111 | + | seen := map[int]bool{} | |
| 112 | + | for _, f := range got { | |
| 113 | + | require.False(t, seen[f], "frame %d assigned twice: %v", f, got) | |
| 114 | + | seen[f] = true | |
| 115 | + | } | |
| 116 | + | require.Equal(t, map[int]int{0: 0}, got) | |
| 117 | + | } | |
| @@ -73,9 +73,9 @@ func TestMatchFramesSingle(t *testing.T) { | |||||
| 73 | func TestMatchFramesActive(t *testing.T) { | 73 | func TestMatchFramesActive(t *testing.T) { | ||
| 74 | frames := []frameInfo{{w: 800, h: 600}, {w: 800, h: 600, active: true}} | 74 | frames := []frameInfo{{w: 800, h: 600}, {w: 800, h: 600, active: true}} | ||
| 75 | got := matchFrames(wins(win(1, 800, 600, true), win(2, 800, 600, false)), frames) | 75 | got := matchFrames(wins(win(1, 800, 600, true), win(2, 800, 600, false)), frames) | ||
| 76 | - | // The focused window takes the ACTIVE frame; the other cannot be told | 76 | + | // The focused window takes the ACTIVE frame and reserves it; the other |
| 77 | - | // apart by size (both frames match it) and is left out. | 77 | + | // window then only has the remaining frame to match. |
| 78 | - | require.Equal(t, map[int]int{0: 1}, got) | 78 | + | require.Equal(t, map[int]int{0: 1, 1: 0}, got) |
| 79 | } | 79 | } | ||
| 80 | 80 | ||||
| 81 | func TestMatchFramesSize(t *testing.T) { | 81 | func TestMatchFramesSize(t *testing.T) { | ||
| @@ -102,3 +102,16 @@ func TestMatchFramesAmbiguous(t *testing.T) { | |||||
| 102 | // No frame close enough. | 102 | // No frame close enough. | ||
| 103 | require.Empty(t, matchFrames(wins(win(1, 200, 200, false), win(2, 300, 300, false)), frames)) | 103 | require.Empty(t, matchFrames(wins(win(1, 200, 200, false), win(2, 300, 300, false)), frames)) | ||
| 104 | } | 104 | } | ||
| 105 | + | ||||
| 106 | + | // The active frame is reserved by the focused window: an unfocused window of | |||
| 107 | + | // the same size as that frame must not be paired with it as well. | |||
| 108 | + | func TestMatchFramesOneToOne(t *testing.T) { | |||
| 109 | + | frames := []frameInfo{{ref: ref{"a", "/active"}, active: true, w: 400, h: 300}, {ref: ref{"a", "/idle"}, w: 800, h: 600}} | |||
| 110 | + | got := matchFrames(wins(win(1, 800, 600, true), win(2, 400, 300, false)), frames) | |||
| 111 | + | seen := map[int]bool{} | |||
| 112 | + | for _, f := range got { | |||
| 113 | + | require.False(t, seen[f], "frame %d assigned twice: %v", f, got) | |||
| 114 | + | seen[f] = true | |||
| 115 | + | } | |||
| 116 | + | require.Equal(t, map[int]int{0: 0}, got) | |||
| 117 | + | } | |||
M internal/adapters/atspi/source.go +136 -15
| @@ -20,22 +20,59 @@ const ( | |||
| 20 | 20 | budget = 300 * time.Millisecond | |
| 21 | 21 | ) | |
| 22 | 22 | ||
| 23 | + | // connectBudget bounds the whole connection setup (session bus, | |
| 24 | + | // GetAddress, IsEnabled, dial of the accessibility bus) of one attempt. | |
| 25 | + | const connectBudget = 300 * time.Millisecond | |
| 26 | + | ||
| 27 | + | // retryEvery is the minimum time between two connection attempts. | |
| 28 | + | const retryEvery = 5 * time.Second | |
| 29 | + | ||
| 30 | + | // connector opens a transport on the accessibility bus. | |
| 31 | + | type connector interface { | |
| 32 | + | Connect(ctx context.Context) (transport, error) | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | // dialResult is the outcome of a connection attempt. | |
| 36 | + | type dialResult struct { | |
| 37 | + | tr transport | |
| 38 | + | err error | |
| 39 | + | } | |
| 40 | + | ||
| 23 | 41 | // Source reads hint targets from the accessibility tree. It owns one | |
| 24 | 42 | // connection to the accessibility bus and is used by one goroutine at a | |
| 25 | - | // time (the hints worker). | |
| 43 | + | // time (the hints worker). The connection is made on the first Targets call | |
| 44 | + | // that needs it, never at startup, and made again after a failure, at most | |
| 45 | + | // once every retryEvery; meanwhile Targets finds nothing and OCR takes over. | |
| 26 | 46 | type Source struct { | |
| 27 | - | tr transport | |
| 47 | + | tr transport | |
| 48 | + | dial connector | |
| 49 | + | // attempt is the outcome of a connection attempt that may still run: a | |
| 50 | + | // silent service is abandoned, not waited for, and its late result is | |
| 51 | + | // closed. | |
| 52 | + | attempt chan dialResult | |
| 53 | + | attemptDue time.Time | |
| 54 | + | retryAt time.Time | |
| 28 | 55 | } | |
| 29 | 56 | ||
| 30 | - | // New enables accessibility on the session and connects to the | |
| 31 | - | // accessibility bus. | |
| 32 | - | func New(ctx context.Context) (*Source, error) { | |
| 57 | + | // New returns a Source. It does not touch the bus: the connection is made by | |
| 58 | + | // the first Targets call. | |
| 59 | + | func New() *Source { return &Source{dial: busConnector{}} } | |
| 60 | + | ||
| 61 | + | // busConnector connects to the session bus, enables accessibility and dials | |
| 62 | + | // the accessibility bus. | |
| 63 | + | type busConnector struct{} | |
| 64 | + | ||
| 65 | + | func (busConnector) Connect(ctx context.Context) (transport, error) { | |
| 33 | 66 | log := logging.For(ctx, "atspi") | |
| 34 | 67 | sess, err := zerobus.SessionBus() | |
| 35 | 68 | if err != nil { | |
| 36 | 69 | return nil, fmt.Errorf("atspi: session bus: %w", err) | |
| 37 | 70 | } | |
| 38 | 71 | defer func() { _ = sess.Close() }() | |
| 72 | + | // Call has no deadline: closing the connection ends the wait for a | |
| 73 | + | // silent service. | |
| 74 | + | stop := time.AfterFunc(connectBudget, func() { _ = sess.Close() }) | |
| 75 | + | defer stop.Stop() | |
| 39 | 76 | ||
| 40 | 77 | sess.NewCall(a11yBusName, a11yBusPath, a11yBusName, "GetAddress", "") | |
| 41 | 78 | m, err := sess.Call() | |
| @@ -70,11 +107,75 @@ func New(ctx context.Context) (*Source, error) { | |||
| 70 | 107 | return nil, err | |
| 71 | 108 | } | |
| 72 | 109 | log.Info().Msg("accessibility bus connected") | |
| 73 | - | return &Source{tr: tr}, nil | |
| 110 | + | return tr, nil | |
| 74 | 111 | } | |
| 75 | 112 | ||
| 76 | - | // Close closes the accessibility bus connection. | |
| 77 | - | func (s *Source) Close() error { return s.tr.Close() } | |
| 113 | + | // ensure returns the connection, making one if none exists and the last | |
| 114 | + | // attempt is old enough. It waits at most connectBudget for an attempt. | |
| 115 | + | func (s *Source) ensure(ctx context.Context) transport { | |
| 116 | + | if s.tr != nil { | |
| 117 | + | return s.tr | |
| 118 | + | } | |
| 119 | + | if s.dial == nil { | |
| 120 | + | return nil | |
| 121 | + | } | |
| 122 | + | now := time.Now() | |
| 123 | + | if s.attempt == nil { | |
| 124 | + | if now.Before(s.retryAt) { | |
| 125 | + | return nil | |
| 126 | + | } | |
| 127 | + | s.retryAt = now.Add(retryEvery) | |
| 128 | + | s.attemptDue = now.Add(connectBudget) | |
| 129 | + | ch := make(chan dialResult, 1) | |
| 130 | + | s.attempt = ch | |
| 131 | + | go func() { | |
| 132 | + | tr, err := s.dial.Connect(ctx) | |
| 133 | + | ch <- dialResult{tr, err} | |
| 134 | + | }() | |
| 135 | + | } | |
| 136 | + | timer := time.NewTimer(time.Until(s.attemptDue)) | |
| 137 | + | defer timer.Stop() | |
| 138 | + | select { | |
| 139 | + | case r := <-s.attempt: | |
| 140 | + | s.attempt = nil | |
| 141 | + | if r.err != nil { | |
| 142 | + | log := logging.For(ctx, "atspi") | |
| 143 | + | log.Warn().Err(r.err).Msg("accessibility hint targets unavailable, using OCR only") | |
| 144 | + | return nil | |
| 145 | + | } | |
| 146 | + | s.tr = r.tr | |
| 147 | + | case <-timer.C: | |
| 148 | + | case <-ctx.Done(): | |
| 149 | + | } | |
| 150 | + | return s.tr | |
| 151 | + | } | |
| 152 | + | ||
| 153 | + | // drop closes a broken connection; the next attempt reconnects. | |
| 154 | + | func (s *Source) drop() { | |
| 155 | + | if s.tr != nil { | |
| 156 | + | _ = s.tr.Close() | |
| 157 | + | s.tr = nil | |
| 158 | + | } | |
| 159 | + | } | |
| 160 | + | ||
| 161 | + | // Close closes the accessibility bus connection, and the one an abandoned | |
| 162 | + | // attempt may still deliver. | |
| 163 | + | func (s *Source) Close() error { | |
| 164 | + | var err error | |
| 165 | + | if s.tr != nil { | |
| 166 | + | err = s.tr.Close() | |
| 167 | + | s.tr = nil | |
| 168 | + | } | |
| 169 | + | if ch := s.attempt; ch != nil { | |
| 170 | + | s.attempt = nil | |
| 171 | + | go func() { | |
| 172 | + | if r := <-ch; r.tr != nil { | |
| 173 | + | _ = r.tr.Close() | |
| 174 | + | } | |
| 175 | + | }() | |
| 176 | + | } | |
| 177 | + | return err | |
| 178 | + | } | |
| 78 | 179 | ||
| 79 | 180 | // appInfo is an application of the registry whose process shows a window. | |
| 80 | 181 | type appInfo struct { | |
| @@ -101,13 +202,6 @@ func (s *Source) Targets(ctx context.Context, windows []ports.HintWindow, linksO | |||
| 101 | 202 | if d, ok := ctx.Deadline(); ok && d.Before(deadline) { | |
| 102 | 203 | deadline = d | |
| 103 | 204 | } | |
| 104 | - | do := func(calls []call) []reply { | |
| 105 | - | if len(calls) == 0 || ctx.Err() != nil { | |
| 106 | - | return make([]reply, len(calls)) | |
| 107 | - | } | |
| 108 | - | return s.tr.Do(deadline, callTimeout, calls) | |
| 109 | - | } | |
| 110 | - | ||
| 111 | 205 | wanted := map[int]bool{} | |
| 112 | 206 | for _, w := range windows { | |
| 113 | 207 | if w.PID > 0 { | |
| @@ -117,6 +211,32 @@ func (s *Source) Targets(ctx context.Context, windows []ports.HintWindow, linksO | |||
| 117 | 211 | if len(wanted) == 0 { | |
| 118 | 212 | return nil, covered, nil | |
| 119 | 213 | } | |
| 214 | + | tr := s.ensure(ctx) | |
| 215 | + | if tr == nil { | |
| 216 | + | return nil, covered, nil | |
| 217 | + | } | |
| 218 | + | broken := false | |
| 219 | + | do := func(calls []call) []reply { | |
| 220 | + | if len(calls) == 0 || ctx.Err() != nil { | |
| 221 | + | return make([]reply, len(calls)) | |
| 222 | + | } | |
| 223 | + | if broken { | |
| 224 | + | out := make([]reply, len(calls)) | |
| 225 | + | for i := range out { | |
| 226 | + | out[i].err = ErrBroken | |
| 227 | + | } | |
| 228 | + | return out | |
| 229 | + | } | |
| 230 | + | out := tr.Do(deadline, callTimeout, calls) | |
| 231 | + | for _, r := range out { | |
| 232 | + | if errors.Is(r.err, ErrBroken) { | |
| 233 | + | broken = true | |
| 234 | + | s.drop() | |
| 235 | + | break | |
| 236 | + | } | |
| 237 | + | } | |
| 238 | + | return out | |
| 239 | + | } | |
| 120 | 240 | ||
| 121 | 241 | // Applications of the registry, kept when their process shows a window. | |
| 122 | 242 | rep := do([]call{{op: opChildren, name: registryName, path: registryRoot}}) | |
| @@ -255,6 +375,7 @@ func (s *Source) elements(do func([]call) []reply, matched []winMatch, linksOnly | |||
| 255 | 375 | if !ok { | |
| 256 | 376 | continue | |
| 257 | 377 | } | |
| 378 | + | t.Window = m.win.ID | |
| 258 | 379 | out = append(out, t) | |
| 259 | 380 | if t.Kind == ports.HintLink { | |
| 260 | 381 | covered[m.win.ID] = true | |
| @@ -20,22 +20,59 @@ const ( | |||||
| 20 | budget = 300 * time.Millisecond | 20 | budget = 300 * time.Millisecond | ||
| 21 | ) | 21 | ) | ||
| 22 | 22 | ||||
| 23 | + | // connectBudget bounds the whole connection setup (session bus, | |||
| 24 | + | // GetAddress, IsEnabled, dial of the accessibility bus) of one attempt. | |||
| 25 | + | const connectBudget = 300 * time.Millisecond | |||
| 26 | + | ||||
| 27 | + | // retryEvery is the minimum time between two connection attempts. | |||
| 28 | + | const retryEvery = 5 * time.Second | |||
| 29 | + | ||||
| 30 | + | // connector opens a transport on the accessibility bus. | |||
| 31 | + | type connector interface { | |||
| 32 | + | Connect(ctx context.Context) (transport, error) | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | // dialResult is the outcome of a connection attempt. | |||
| 36 | + | type dialResult struct { | |||
| 37 | + | tr transport | |||
| 38 | + | err error | |||
| 39 | + | } | |||
| 40 | + | ||||
| 23 | // Source reads hint targets from the accessibility tree. It owns one | 41 | // Source reads hint targets from the accessibility tree. It owns one | ||
| 24 | // connection to the accessibility bus and is used by one goroutine at a | 42 | // connection to the accessibility bus and is used by one goroutine at a | ||
| 25 | - | // time (the hints worker). | 43 | + | // time (the hints worker). The connection is made on the first Targets call |
| 44 | + | // that needs it, never at startup, and made again after a failure, at most | |||
| 45 | + | // once every retryEvery; meanwhile Targets finds nothing and OCR takes over. | |||
| 26 | type Source struct { | 46 | type Source struct { | ||
| 27 | - | tr transport | 47 | + | tr transport |
| 48 | + | dial connector | |||
| 49 | + | // attempt is the outcome of a connection attempt that may still run: a | |||
| 50 | + | // silent service is abandoned, not waited for, and its late result is | |||
| 51 | + | // closed. | |||
| 52 | + | attempt chan dialResult | |||
| 53 | + | attemptDue time.Time | |||
| 54 | + | retryAt time.Time | |||
| 28 | } | 55 | } | ||
| 29 | 56 | ||||
| 30 | - | // New enables accessibility on the session and connects to the | 57 | + | // New returns a Source. It does not touch the bus: the connection is made by |
| 31 | - | // accessibility bus. | 58 | + | // the first Targets call. |
| 32 | - | func New(ctx context.Context) (*Source, error) { | 59 | + | func New() *Source { return &Source{dial: busConnector{}} } |
| 60 | + | ||||
| 61 | + | // busConnector connects to the session bus, enables accessibility and dials | |||
| 62 | + | // the accessibility bus. | |||
| 63 | + | type busConnector struct{} | |||
| 64 | + | ||||
| 65 | + | func (busConnector) Connect(ctx context.Context) (transport, error) { | |||
| 33 | log := logging.For(ctx, "atspi") | 66 | log := logging.For(ctx, "atspi") | ||
| 34 | sess, err := zerobus.SessionBus() | 67 | sess, err := zerobus.SessionBus() | ||
| 35 | if err != nil { | 68 | if err != nil { | ||
| 36 | return nil, fmt.Errorf("atspi: session bus: %w", err) | 69 | return nil, fmt.Errorf("atspi: session bus: %w", err) | ||
| 37 | } | 70 | } | ||
| 38 | defer func() { _ = sess.Close() }() | 71 | defer func() { _ = sess.Close() }() | ||
| 72 | + | // Call has no deadline: closing the connection ends the wait for a | |||
| 73 | + | // silent service. | |||
| 74 | + | stop := time.AfterFunc(connectBudget, func() { _ = sess.Close() }) | |||
| 75 | + | defer stop.Stop() | |||
| 39 | 76 | ||||
| 40 | sess.NewCall(a11yBusName, a11yBusPath, a11yBusName, "GetAddress", "") | 77 | sess.NewCall(a11yBusName, a11yBusPath, a11yBusName, "GetAddress", "") | ||
| 41 | m, err := sess.Call() | 78 | m, err := sess.Call() | ||
| @@ -70,11 +107,75 @@ func New(ctx context.Context) (*Source, error) { | |||||
| 70 | return nil, err | 107 | return nil, err | ||
| 71 | } | 108 | } | ||
| 72 | log.Info().Msg("accessibility bus connected") | 109 | log.Info().Msg("accessibility bus connected") | ||
| 73 | - | return &Source{tr: tr}, nil | 110 | + | return tr, nil |
| 74 | } | 111 | } | ||
| 75 | 112 | ||||
| 76 | - | // Close closes the accessibility bus connection. | 113 | + | // ensure returns the connection, making one if none exists and the last |
| 77 | - | func (s *Source) Close() error { return s.tr.Close() } | 114 | + | // attempt is old enough. It waits at most connectBudget for an attempt. |
| 115 | + | func (s *Source) ensure(ctx context.Context) transport { | |||
| 116 | + | if s.tr != nil { | |||
| 117 | + | return s.tr | |||
| 118 | + | } | |||
| 119 | + | if s.dial == nil { | |||
| 120 | + | return nil | |||
| 121 | + | } | |||
| 122 | + | now := time.Now() | |||
| 123 | + | if s.attempt == nil { | |||
| 124 | + | if now.Before(s.retryAt) { | |||
| 125 | + | return nil | |||
| 126 | + | } | |||
| 127 | + | s.retryAt = now.Add(retryEvery) | |||
| 128 | + | s.attemptDue = now.Add(connectBudget) | |||
| 129 | + | ch := make(chan dialResult, 1) | |||
| 130 | + | s.attempt = ch | |||
| 131 | + | go func() { | |||
| 132 | + | tr, err := s.dial.Connect(ctx) | |||
| 133 | + | ch <- dialResult{tr, err} | |||
| 134 | + | }() | |||
| 135 | + | } | |||
| 136 | + | timer := time.NewTimer(time.Until(s.attemptDue)) | |||
| 137 | + | defer timer.Stop() | |||
| 138 | + | select { | |||
| 139 | + | case r := <-s.attempt: | |||
| 140 | + | s.attempt = nil | |||
| 141 | + | if r.err != nil { | |||
| 142 | + | log := logging.For(ctx, "atspi") | |||
| 143 | + | log.Warn().Err(r.err).Msg("accessibility hint targets unavailable, using OCR only") | |||
| 144 | + | return nil | |||
| 145 | + | } | |||
| 146 | + | s.tr = r.tr | |||
| 147 | + | case <-timer.C: | |||
| 148 | + | case <-ctx.Done(): | |||
| 149 | + | } | |||
| 150 | + | return s.tr | |||
| 151 | + | } | |||
| 152 | + | ||||
| 153 | + | // drop closes a broken connection; the next attempt reconnects. | |||
| 154 | + | func (s *Source) drop() { | |||
| 155 | + | if s.tr != nil { | |||
| 156 | + | _ = s.tr.Close() | |||
| 157 | + | s.tr = nil | |||
| 158 | + | } | |||
| 159 | + | } | |||
| 160 | + | ||||
| 161 | + | // Close closes the accessibility bus connection, and the one an abandoned | |||
| 162 | + | // attempt may still deliver. | |||
| 163 | + | func (s *Source) Close() error { | |||
| 164 | + | var err error | |||
| 165 | + | if s.tr != nil { | |||
| 166 | + | err = s.tr.Close() | |||
| 167 | + | s.tr = nil | |||
| 168 | + | } | |||
| 169 | + | if ch := s.attempt; ch != nil { | |||
| 170 | + | s.attempt = nil | |||
| 171 | + | go func() { | |||
| 172 | + | if r := <-ch; r.tr != nil { | |||
| 173 | + | _ = r.tr.Close() | |||
| 174 | + | } | |||
| 175 | + | }() | |||
| 176 | + | } | |||
| 177 | + | return err | |||
| 178 | + | } | |||
| 78 | 179 | ||||
| 79 | // appInfo is an application of the registry whose process shows a window. | 180 | // appInfo is an application of the registry whose process shows a window. | ||
| 80 | type appInfo struct { | 181 | type appInfo struct { | ||
| @@ -101,13 +202,6 @@ func (s *Source) Targets(ctx context.Context, windows []ports.HintWindow, linksO | |||||
| 101 | if d, ok := ctx.Deadline(); ok && d.Before(deadline) { | 202 | if d, ok := ctx.Deadline(); ok && d.Before(deadline) { | ||
| 102 | deadline = d | 203 | deadline = d | ||
| 103 | } | 204 | } | ||
| 104 | - | do := func(calls []call) []reply { | |||
| 105 | - | if len(calls) == 0 || ctx.Err() != nil { | |||
| 106 | - | return make([]reply, len(calls)) | |||
| 107 | - | } | |||
| 108 | - | return s.tr.Do(deadline, callTimeout, calls) | |||
| 109 | - | } | |||
| 110 | - | ||||
| 111 | wanted := map[int]bool{} | 205 | wanted := map[int]bool{} | ||
| 112 | for _, w := range windows { | 206 | for _, w := range windows { | ||
| 113 | if w.PID > 0 { | 207 | if w.PID > 0 { | ||
| @@ -117,6 +211,32 @@ func (s *Source) Targets(ctx context.Context, windows []ports.HintWindow, linksO | |||||
| 117 | if len(wanted) == 0 { | 211 | if len(wanted) == 0 { | ||
| 118 | return nil, covered, nil | 212 | return nil, covered, nil | ||
| 119 | } | 213 | } | ||
| 214 | + | tr := s.ensure(ctx) | |||
| 215 | + | if tr == nil { | |||
| 216 | + | return nil, covered, nil | |||
| 217 | + | } | |||
| 218 | + | broken := false | |||
| 219 | + | do := func(calls []call) []reply { | |||
| 220 | + | if len(calls) == 0 || ctx.Err() != nil { | |||
| 221 | + | return make([]reply, len(calls)) | |||
| 222 | + | } | |||
| 223 | + | if broken { | |||
| 224 | + | out := make([]reply, len(calls)) | |||
| 225 | + | for i := range out { | |||
| 226 | + | out[i].err = ErrBroken | |||
| 227 | + | } | |||
| 228 | + | return out | |||
| 229 | + | } | |||
| 230 | + | out := tr.Do(deadline, callTimeout, calls) | |||
| 231 | + | for _, r := range out { | |||
| 232 | + | if errors.Is(r.err, ErrBroken) { | |||
| 233 | + | broken = true | |||
| 234 | + | s.drop() | |||
| 235 | + | break | |||
| 236 | + | } | |||
| 237 | + | } | |||
| 238 | + | return out | |||
| 239 | + | } | |||
| 120 | 240 | ||||
| 121 | // Applications of the registry, kept when their process shows a window. | 241 | // Applications of the registry, kept when their process shows a window. | ||
| 122 | rep := do([]call{{op: opChildren, name: registryName, path: registryRoot}}) | 242 | rep := do([]call{{op: opChildren, name: registryName, path: registryRoot}}) | ||
| @@ -255,6 +375,7 @@ func (s *Source) elements(do func([]call) []reply, matched []winMatch, linksOnly | |||||
| 255 | if !ok { | 375 | if !ok { | ||
| 256 | continue | 376 | continue | ||
| 257 | } | 377 | } | ||
| 378 | + | t.Window = m.win.ID | |||
| 258 | out = append(out, t) | 379 | out = append(out, t) | ||
| 259 | if t.Kind == ports.HintLink { | 380 | if t.Kind == ports.HintLink { | ||
| 260 | covered[m.win.ID] = true | 381 | covered[m.win.ID] = true | ||
M internal/adapters/atspi/source_test.go +94 -2
| @@ -2,6 +2,7 @@ package atspi | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "errors" | |
| 5 | 6 | "fmt" | |
| 6 | 7 | "testing" | |
| 7 | 8 | "time" | |
| @@ -74,8 +75,8 @@ func TestTargetsCoversWindowAndConverts(t *testing.T) { | |||
| 74 | 75 | require.NoError(t, err) | |
| 75 | 76 | require.Equal(t, map[ports.WindowID]bool{1: true}, covered) | |
| 76 | 77 | require.Equal(t, []ports.HintTarget{ | |
| 77 | - | {Rect: ports.Rect{X: 110, Y: 300, W: 50, H: 16}, Kind: ports.HintLink}, | |
| 78 | - | {Rect: ports.Rect{X: 200, Y: 400, W: 20, H: 20}, Kind: ports.HintButton}, | |
| 78 | + | {Rect: ports.Rect{X: 110, Y: 300, W: 50, H: 16}, Kind: ports.HintLink, Window: 1}, | |
| 79 | + | {Rect: ports.Rect{X: 200, Y: 400, W: 20, H: 20}, Kind: ports.HintButton, Window: 1}, | |
| 79 | 80 | }, targets) | |
| 80 | 81 | } | |
| 81 | 82 | ||
| @@ -144,3 +145,94 @@ func TestCoverageRequiresLinkOrDocument(t *testing.T) { | |||
| 144 | 145 | } | |
| 145 | 146 | } | |
| 146 | 147 | } | |
| 148 | + | ||
| 149 | + | var oneWindow = []ports.HintWindow{{ID: 1, PID: 77, Rect: ports.Rect{W: 800, H: 600}}} | |
| 150 | + | ||
| 151 | + | // New does not touch the bus; the first Targets call connects, once. | |
| 152 | + | func TestConnectsLazilyOnFirstTargets(t *testing.T) { | |
| 153 | + | tr := newMocktransport(t) | |
| 154 | + | tr.EXPECT().Do(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(answerTree( | |
| 155 | + | [4]int32{0, 0, 800, 600}, []ref{{":1.5", "/a"}}, map[string][4]int32{"/a": {1, 1, 9, 9}}, map[string]uint32{"/a": roleLink}, false)) | |
| 156 | + | c := newMockconnector(t) | |
| 157 | + | c.EXPECT().Connect(mock.Anything).Return(tr, nil).Once() | |
| 158 | + | s := &Source{dial: c} | |
| 159 | + | for range 2 { | |
| 160 | + | targets, _, err := s.Targets(context.Background(), oneWindow, false) | |
| 161 | + | require.NoError(t, err) | |
| 162 | + | require.Len(t, targets, 1) | |
| 163 | + | } | |
| 164 | + | } | |
| 165 | + | ||
| 166 | + | // Without PIDs nothing is asked and nothing is connected. | |
| 167 | + | func TestNoConnectWithoutWindows(t *testing.T) { | |
| 168 | + | s := &Source{dial: newMockconnector(t)} | |
| 169 | + | _, _, err := s.Targets(context.Background(), []ports.HintWindow{{ID: 1}}, false) | |
| 170 | + | require.NoError(t, err) | |
| 171 | + | } | |
| 172 | + | ||
| 173 | + | // A failed connection is retried no sooner than retryEvery; meanwhile no | |
| 174 | + | // targets are returned. | |
| 175 | + | func TestConnectRetriesWithBackoff(t *testing.T) { | |
| 176 | + | c := newMockconnector(t) | |
| 177 | + | c.EXPECT().Connect(mock.Anything).Return(nil, errors.New("down")).Once() | |
| 178 | + | s := &Source{dial: c} | |
| 179 | + | for range 3 { | |
| 180 | + | targets, covered, err := s.Targets(context.Background(), oneWindow, false) | |
| 181 | + | require.NoError(t, err) | |
| 182 | + | require.Empty(t, targets) | |
| 183 | + | require.Empty(t, covered) | |
| 184 | + | } | |
| 185 | + | tr := newMocktransport(t) | |
| 186 | + | tr.EXPECT().Do(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(answerTree( | |
| 187 | + | [4]int32{0, 0, 800, 600}, []ref{{":1.5", "/a"}}, map[string][4]int32{"/a": {1, 1, 9, 9}}, map[string]uint32{"/a": roleLink}, false)) | |
| 188 | + | c.EXPECT().Connect(mock.Anything).Return(tr, nil).Once() | |
| 189 | + | s.retryAt = time.Now().Add(-time.Second) | |
| 190 | + | targets, _, err := s.Targets(context.Background(), oneWindow, false) | |
| 191 | + | require.NoError(t, err) | |
| 192 | + | require.Len(t, targets, 1) | |
| 193 | + | } | |
| 194 | + | ||
| 195 | + | // A broken connection is closed and dropped; the next attempt reconnects. | |
| 196 | + | func TestBrokenConnectionIsDropped(t *testing.T) { | |
| 197 | + | tr := newMocktransport(t) | |
| 198 | + | tr.EXPECT().Do(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |
| 199 | + | func(_ time.Time, _ time.Duration, calls []call) []reply { | |
| 200 | + | out := make([]reply, len(calls)) | |
| 201 | + | for i := range out { | |
| 202 | + | out[i].err = ErrBroken | |
| 203 | + | } | |
| 204 | + | return out | |
| 205 | + | }).Once() | |
| 206 | + | tr.EXPECT().Close().Return(nil).Once() | |
| 207 | + | c := newMockconnector(t) | |
| 208 | + | c.EXPECT().Connect(mock.Anything).Return(tr, nil).Once() | |
| 209 | + | s := &Source{dial: c} | |
| 210 | + | _, _, err := s.Targets(context.Background(), oneWindow, false) | |
| 211 | + | require.Error(t, err) | |
| 212 | + | require.Nil(t, s.tr) | |
| 213 | + | targets, _, err := s.Targets(context.Background(), oneWindow, false) | |
| 214 | + | require.NoError(t, err) | |
| 215 | + | require.Empty(t, targets) // backoff: no new attempt yet | |
| 216 | + | } | |
| 217 | + | ||
| 218 | + | // A silent service cannot hold Targets beyond the connect budget. | |
| 219 | + | func TestConnectIsBounded(t *testing.T) { | |
| 220 | + | release := make(chan struct{}) | |
| 221 | + | tr := newMocktransport(t) | |
| 222 | + | tr.EXPECT().Close().Return(nil).Once() | |
| 223 | + | c := newMockconnector(t) | |
| 224 | + | c.EXPECT().Connect(mock.Anything).RunAndReturn(func(context.Context) (transport, error) { | |
| 225 | + | <-release | |
| 226 | + | return tr, nil | |
| 227 | + | }).Once() | |
| 228 | + | s := &Source{dial: c} | |
| 229 | + | t0 := time.Now() | |
| 230 | + | targets, _, err := s.Targets(context.Background(), oneWindow, false) | |
| 231 | + | require.NoError(t, err) | |
| 232 | + | require.Empty(t, targets) | |
| 233 | + | require.Less(t, time.Since(t0), connectBudget+200*time.Millisecond) | |
| 234 | + | // The late result is closed with the source. | |
| 235 | + | require.NoError(t, s.Close()) | |
| 236 | + | close(release) | |
| 237 | + | require.Eventually(t, func() bool { return tr.AssertExpectations(&testing.T{}) }, time.Second, 5*time.Millisecond) | |
| 238 | + | } | |
| @@ -2,6 +2,7 @@ package atspi | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "errors" | |||
| 5 | "fmt" | 6 | "fmt" | ||
| 6 | "testing" | 7 | "testing" | ||
| 7 | "time" | 8 | "time" | ||
| @@ -74,8 +75,8 @@ func TestTargetsCoversWindowAndConverts(t *testing.T) { | |||||
| 74 | require.NoError(t, err) | 75 | require.NoError(t, err) | ||
| 75 | require.Equal(t, map[ports.WindowID]bool{1: true}, covered) | 76 | require.Equal(t, map[ports.WindowID]bool{1: true}, covered) | ||
| 76 | require.Equal(t, []ports.HintTarget{ | 77 | require.Equal(t, []ports.HintTarget{ | ||
| 77 | - | {Rect: ports.Rect{X: 110, Y: 300, W: 50, H: 16}, Kind: ports.HintLink}, | 78 | + | {Rect: ports.Rect{X: 110, Y: 300, W: 50, H: 16}, Kind: ports.HintLink, Window: 1}, |
| 78 | - | {Rect: ports.Rect{X: 200, Y: 400, W: 20, H: 20}, Kind: ports.HintButton}, | 79 | + | {Rect: ports.Rect{X: 200, Y: 400, W: 20, H: 20}, Kind: ports.HintButton, Window: 1}, |
| 79 | }, targets) | 80 | }, targets) | ||
| 80 | } | 81 | } | ||
| 81 | 82 | ||||
| @@ -144,3 +145,94 @@ func TestCoverageRequiresLinkOrDocument(t *testing.T) { | |||||
| 144 | } | 145 | } | ||
| 145 | } | 146 | } | ||
| 146 | } | 147 | } | ||
| 148 | + | ||||
| 149 | + | var oneWindow = []ports.HintWindow{{ID: 1, PID: 77, Rect: ports.Rect{W: 800, H: 600}}} | |||
| 150 | + | ||||
| 151 | + | // New does not touch the bus; the first Targets call connects, once. | |||
| 152 | + | func TestConnectsLazilyOnFirstTargets(t *testing.T) { | |||
| 153 | + | tr := newMocktransport(t) | |||
| 154 | + | tr.EXPECT().Do(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(answerTree( | |||
| 155 | + | [4]int32{0, 0, 800, 600}, []ref{{":1.5", "/a"}}, map[string][4]int32{"/a": {1, 1, 9, 9}}, map[string]uint32{"/a": roleLink}, false)) | |||
| 156 | + | c := newMockconnector(t) | |||
| 157 | + | c.EXPECT().Connect(mock.Anything).Return(tr, nil).Once() | |||
| 158 | + | s := &Source{dial: c} | |||
| 159 | + | for range 2 { | |||
| 160 | + | targets, _, err := s.Targets(context.Background(), oneWindow, false) | |||
| 161 | + | require.NoError(t, err) | |||
| 162 | + | require.Len(t, targets, 1) | |||
| 163 | + | } | |||
| 164 | + | } | |||
| 165 | + | ||||
| 166 | + | // Without PIDs nothing is asked and nothing is connected. | |||
| 167 | + | func TestNoConnectWithoutWindows(t *testing.T) { | |||
| 168 | + | s := &Source{dial: newMockconnector(t)} | |||
| 169 | + | _, _, err := s.Targets(context.Background(), []ports.HintWindow{{ID: 1}}, false) | |||
| 170 | + | require.NoError(t, err) | |||
| 171 | + | } | |||
| 172 | + | ||||
| 173 | + | // A failed connection is retried no sooner than retryEvery; meanwhile no | |||
| 174 | + | // targets are returned. | |||
| 175 | + | func TestConnectRetriesWithBackoff(t *testing.T) { | |||
| 176 | + | c := newMockconnector(t) | |||
| 177 | + | c.EXPECT().Connect(mock.Anything).Return(nil, errors.New("down")).Once() | |||
| 178 | + | s := &Source{dial: c} | |||
| 179 | + | for range 3 { | |||
| 180 | + | targets, covered, err := s.Targets(context.Background(), oneWindow, false) | |||
| 181 | + | require.NoError(t, err) | |||
| 182 | + | require.Empty(t, targets) | |||
| 183 | + | require.Empty(t, covered) | |||
| 184 | + | } | |||
| 185 | + | tr := newMocktransport(t) | |||
| 186 | + | tr.EXPECT().Do(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(answerTree( | |||
| 187 | + | [4]int32{0, 0, 800, 600}, []ref{{":1.5", "/a"}}, map[string][4]int32{"/a": {1, 1, 9, 9}}, map[string]uint32{"/a": roleLink}, false)) | |||
| 188 | + | c.EXPECT().Connect(mock.Anything).Return(tr, nil).Once() | |||
| 189 | + | s.retryAt = time.Now().Add(-time.Second) | |||
| 190 | + | targets, _, err := s.Targets(context.Background(), oneWindow, false) | |||
| 191 | + | require.NoError(t, err) | |||
| 192 | + | require.Len(t, targets, 1) | |||
| 193 | + | } | |||
| 194 | + | ||||
| 195 | + | // A broken connection is closed and dropped; the next attempt reconnects. | |||
| 196 | + | func TestBrokenConnectionIsDropped(t *testing.T) { | |||
| 197 | + | tr := newMocktransport(t) | |||
| 198 | + | tr.EXPECT().Do(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |||
| 199 | + | func(_ time.Time, _ time.Duration, calls []call) []reply { | |||
| 200 | + | out := make([]reply, len(calls)) | |||
| 201 | + | for i := range out { | |||
| 202 | + | out[i].err = ErrBroken | |||
| 203 | + | } | |||
| 204 | + | return out | |||
| 205 | + | }).Once() | |||
| 206 | + | tr.EXPECT().Close().Return(nil).Once() | |||
| 207 | + | c := newMockconnector(t) | |||
| 208 | + | c.EXPECT().Connect(mock.Anything).Return(tr, nil).Once() | |||
| 209 | + | s := &Source{dial: c} | |||
| 210 | + | _, _, err := s.Targets(context.Background(), oneWindow, false) | |||
| 211 | + | require.Error(t, err) | |||
| 212 | + | require.Nil(t, s.tr) | |||
| 213 | + | targets, _, err := s.Targets(context.Background(), oneWindow, false) | |||
| 214 | + | require.NoError(t, err) | |||
| 215 | + | require.Empty(t, targets) // backoff: no new attempt yet | |||
| 216 | + | } | |||
| 217 | + | ||||
| 218 | + | // A silent service cannot hold Targets beyond the connect budget. | |||
| 219 | + | func TestConnectIsBounded(t *testing.T) { | |||
| 220 | + | release := make(chan struct{}) | |||
| 221 | + | tr := newMocktransport(t) | |||
| 222 | + | tr.EXPECT().Close().Return(nil).Once() | |||
| 223 | + | c := newMockconnector(t) | |||
| 224 | + | c.EXPECT().Connect(mock.Anything).RunAndReturn(func(context.Context) (transport, error) { | |||
| 225 | + | <-release | |||
| 226 | + | return tr, nil | |||
| 227 | + | }).Once() | |||
| 228 | + | s := &Source{dial: c} | |||
| 229 | + | t0 := time.Now() | |||
| 230 | + | targets, _, err := s.Targets(context.Background(), oneWindow, false) | |||
| 231 | + | require.NoError(t, err) | |||
| 232 | + | require.Empty(t, targets) | |||
| 233 | + | require.Less(t, time.Since(t0), connectBudget+200*time.Millisecond) | |||
| 234 | + | // The late result is closed with the source. | |||
| 235 | + | require.NoError(t, s.Close()) | |||
| 236 | + | close(release) | |||
| 237 | + | require.Eventually(t, func() bool { return tr.AssertExpectations(&testing.T{}) }, time.Second, 5*time.Millisecond) | |||
| 238 | + | } | |||
M internal/adapters/atspi/transport.go +70 -18
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "errors" | |
| 5 | 5 | "fmt" | |
| 6 | 6 | "strings" | |
| 7 | + | "sync/atomic" | |
| 7 | 8 | "syscall" | |
| 8 | 9 | "time" | |
| 9 | 10 | ||
| @@ -75,8 +76,10 @@ type reply struct { | |||
| 75 | 76 | // goroutine at a time. | |
| 76 | 77 | type transport interface { | |
| 77 | 78 | // Do sends calls pipelined and returns one reply per call, in order. | |
| 78 | - | // The batch ends at deadline; it also ends when no reply arrives for | |
| 79 | - | // idle. Calls still unanswered then get ErrTimeout. | |
| 79 | + | // The batch ends at deadline. A call not answered idle after it was | |
| 80 | + | // sent gets ErrTimeout. When the connection fails or the deadline | |
| 81 | + | // finds calls unanswered, those calls get an error wrapping | |
| 82 | + | // ErrBroken: the connection must be dropped. | |
| 80 | 83 | Do(deadline time.Time, idle time.Duration, calls []call) []reply | |
| 81 | 84 | Close() error | |
| 82 | 85 | } | |
| @@ -136,13 +139,46 @@ func (b *conn) readable(d time.Duration) (bool, error) { | |||
| 136 | 139 | return n > 0, perr | |
| 137 | 140 | } | |
| 138 | 141 | ||
| 142 | + | // ErrBroken is the error of calls on a connection that failed or was closed | |
| 143 | + | // at the deadline; the owner must drop it and connect again. | |
| 144 | + | var ErrBroken = errors.New("atspi: connection broken") | |
| 145 | + | ||
| 146 | + | // errDeadline is the error of calls still unanswered at the batch deadline: | |
| 147 | + | // late replies and half-read messages leave the connection unusable. | |
| 148 | + | var errDeadline = fmt.Errorf("%w: %w", ErrBroken, ErrTimeout) | |
| 149 | + | ||
| 150 | + | // watchdogGrace is how long after the deadline the watchdog closes a socket | |
| 151 | + | // that the batch loop could not leave by itself (a stuck read or write). | |
| 152 | + | const watchdogGrace = 20 * time.Millisecond | |
| 153 | + | ||
| 154 | + | // Do sends the calls pipelined. A watchdog closes the socket shortly after | |
| 155 | + | // deadline, so neither a blocked write nor a read stuck in the middle of a | |
| 156 | + | // message can outlive the batch; the connection is then broken. Each call | |
| 157 | + | // has its own idle timeout, counted from its send: traffic for other calls | |
| 158 | + | // does not extend it. | |
| 139 | 159 | func (b *conn) Do(deadline time.Time, idle time.Duration, calls []call) []reply { | |
| 140 | 160 | out := make([]reply, len(calls)) | |
| 141 | 161 | answered := make([]bool, len(calls)) | |
| 142 | - | pending := make(map[uint32]int, min(len(calls), maxInflight)) | |
| 162 | + | type inflight struct { | |
| 163 | + | idx int | |
| 164 | + | sent time.Time | |
| 165 | + | } | |
| 166 | + | pending := make(map[uint32]inflight, min(len(calls), maxInflight)) | |
| 143 | 167 | next, done := 0, 0 | |
| 168 | + | var fired atomic.Bool | |
| 169 | + | watchdog := time.AfterFunc(time.Until(deadline)+watchdogGrace, func() { | |
| 170 | + | fired.Store(true) | |
| 171 | + | _ = b.c.Close() | |
| 172 | + | }) | |
| 173 | + | defer watchdog.Stop() | |
| 144 | 174 | // finish gives err to every call without an answer. | |
| 145 | 175 | finish := func(err error) []reply { | |
| 176 | + | if !errors.Is(err, ErrBroken) && (fired.Load() || !errors.Is(err, ErrTimeout)) { | |
| 177 | + | err = fmt.Errorf("%w: %w", ErrBroken, err) | |
| 178 | + | } | |
| 179 | + | if errors.Is(err, ErrBroken) { | |
| 180 | + | _ = b.c.Close() // late replies would desynchronize the next batch | |
| 181 | + | } | |
| 146 | 182 | for i := range out { | |
| 147 | 183 | if !answered[i] { | |
| 148 | 184 | out[i].err = err | |
| @@ -152,48 +188,64 @@ func (b *conn) Do(deadline time.Time, idle time.Duration, calls []call) []reply | |||
| 152 | 188 | } | |
| 153 | 189 | for done < len(calls) { | |
| 154 | 190 | for next < len(calls) && len(pending) < maxInflight { | |
| 191 | + | if !time.Now().Before(deadline) { | |
| 192 | + | return finish(errDeadline) | |
| 193 | + | } | |
| 155 | 194 | serial, err := b.send(calls[next]) | |
| 156 | 195 | if err != nil { | |
| 157 | - | if errors.Is(err, zerobus.ErrClosed) { | |
| 158 | - | return finish(err) | |
| 159 | - | } | |
| 160 | - | out[next].err = err | |
| 161 | - | answered[next] = true | |
| 162 | - | done++ | |
| 163 | - | } else { | |
| 164 | - | pending[serial] = next | |
| 196 | + | return finish(err) | |
| 165 | 197 | } | |
| 198 | + | pending[serial] = inflight{idx: next, sent: time.Now()} | |
| 166 | 199 | next++ | |
| 167 | 200 | } | |
| 201 | + | // Calls that waited longer than idle get no reply. | |
| 202 | + | now := time.Now() | |
| 203 | + | var oldest time.Time | |
| 204 | + | for serial, p := range pending { | |
| 205 | + | if now.Sub(p.sent) >= idle { | |
| 206 | + | out[p.idx].err = ErrTimeout | |
| 207 | + | answered[p.idx] = true | |
| 208 | + | done++ | |
| 209 | + | delete(pending, serial) | |
| 210 | + | } else if oldest.IsZero() || p.sent.Before(oldest) { | |
| 211 | + | oldest = p.sent | |
| 212 | + | } | |
| 213 | + | } | |
| 168 | 214 | if len(pending) == 0 { | |
| 169 | 215 | continue | |
| 170 | 216 | } | |
| 171 | - | wait := min(idle, time.Until(deadline)) | |
| 217 | + | wait := min(time.Until(oldest.Add(idle)), time.Until(deadline)) | |
| 172 | 218 | if wait <= 0 { | |
| 173 | - | break | |
| 219 | + | if !time.Now().Before(deadline) { | |
| 220 | + | break | |
| 221 | + | } | |
| 222 | + | continue | |
| 174 | 223 | } | |
| 175 | 224 | ok, err := b.readable(wait) | |
| 176 | 225 | if err != nil { | |
| 177 | 226 | return finish(err) | |
| 178 | 227 | } | |
| 179 | 228 | if !ok { | |
| 180 | - | break | |
| 229 | + | continue | |
| 181 | 230 | } | |
| 182 | 231 | m, err := b.c.ReadMessage() | |
| 183 | 232 | if err != nil { | |
| 184 | 233 | return finish(err) | |
| 185 | 234 | } | |
| 186 | - | i, found := pending[m.ReplySerial] | |
| 235 | + | p, found := pending[m.ReplySerial] | |
| 187 | 236 | if !found || (m.Type != zerobus.TypeMethodReturn && m.Type != zerobus.TypeError) { | |
| 188 | 237 | // A signal, or the late reply of an earlier batch. | |
| 189 | 238 | continue | |
| 190 | 239 | } | |
| 191 | 240 | delete(pending, m.ReplySerial) | |
| 192 | - | decode(m, calls[i], &out[i]) | |
| 193 | - | answered[i] = true | |
| 241 | + | decode(m, calls[p.idx], &out[p.idx]) | |
| 242 | + | answered[p.idx] = true | |
| 194 | 243 | done++ | |
| 195 | 244 | } | |
| 196 | - | return finish(ErrTimeout) | |
| 245 | + | if done == len(calls) { | |
| 246 | + | return out | |
| 247 | + | } | |
| 248 | + | return finish(errDeadline) | |
| 197 | 249 | } | |
| 198 | 250 | ||
| 199 | 251 | // send builds and sends one call and returns its serial. | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "errors" | 4 | "errors" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "strings" | 6 | "strings" | ||
| 7 | + | "sync/atomic" | |||
| 7 | "syscall" | 8 | "syscall" | ||
| 8 | "time" | 9 | "time" | ||
| 9 | 10 | ||||
| @@ -75,8 +76,10 @@ type reply struct { | |||||
| 75 | // goroutine at a time. | 76 | // goroutine at a time. | ||
| 76 | type transport interface { | 77 | type transport interface { | ||
| 77 | // Do sends calls pipelined and returns one reply per call, in order. | 78 | // Do sends calls pipelined and returns one reply per call, in order. | ||
| 78 | - | // The batch ends at deadline; it also ends when no reply arrives for | 79 | + | // The batch ends at deadline. A call not answered idle after it was |
| 79 | - | // idle. Calls still unanswered then get ErrTimeout. | 80 | + | // sent gets ErrTimeout. When the connection fails or the deadline |
| 81 | + | // finds calls unanswered, those calls get an error wrapping | |||
| 82 | + | // ErrBroken: the connection must be dropped. | |||
| 80 | Do(deadline time.Time, idle time.Duration, calls []call) []reply | 83 | Do(deadline time.Time, idle time.Duration, calls []call) []reply | ||
| 81 | Close() error | 84 | Close() error | ||
| 82 | } | 85 | } | ||
| @@ -136,13 +139,46 @@ func (b *conn) readable(d time.Duration) (bool, error) { | |||||
| 136 | return n > 0, perr | 139 | return n > 0, perr | ||
| 137 | } | 140 | } | ||
| 138 | 141 | ||||
| 142 | + | // ErrBroken is the error of calls on a connection that failed or was closed | |||
| 143 | + | // at the deadline; the owner must drop it and connect again. | |||
| 144 | + | var ErrBroken = errors.New("atspi: connection broken") | |||
| 145 | + | ||||
| 146 | + | // errDeadline is the error of calls still unanswered at the batch deadline: | |||
| 147 | + | // late replies and half-read messages leave the connection unusable. | |||
| 148 | + | var errDeadline = fmt.Errorf("%w: %w", ErrBroken, ErrTimeout) | |||
| 149 | + | ||||
| 150 | + | // watchdogGrace is how long after the deadline the watchdog closes a socket | |||
| 151 | + | // that the batch loop could not leave by itself (a stuck read or write). | |||
| 152 | + | const watchdogGrace = 20 * time.Millisecond | |||
| 153 | + | ||||
| 154 | + | // Do sends the calls pipelined. A watchdog closes the socket shortly after | |||
| 155 | + | // deadline, so neither a blocked write nor a read stuck in the middle of a | |||
| 156 | + | // message can outlive the batch; the connection is then broken. Each call | |||
| 157 | + | // has its own idle timeout, counted from its send: traffic for other calls | |||
| 158 | + | // does not extend it. | |||
| 139 | func (b *conn) Do(deadline time.Time, idle time.Duration, calls []call) []reply { | 159 | func (b *conn) Do(deadline time.Time, idle time.Duration, calls []call) []reply { | ||
| 140 | out := make([]reply, len(calls)) | 160 | out := make([]reply, len(calls)) | ||
| 141 | answered := make([]bool, len(calls)) | 161 | answered := make([]bool, len(calls)) | ||
| 142 | - | pending := make(map[uint32]int, min(len(calls), maxInflight)) | 162 | + | type inflight struct { |
| 163 | + | idx int | |||
| 164 | + | sent time.Time | |||
| 165 | + | } | |||
| 166 | + | pending := make(map[uint32]inflight, min(len(calls), maxInflight)) | |||
| 143 | next, done := 0, 0 | 167 | next, done := 0, 0 | ||
| 168 | + | var fired atomic.Bool | |||
| 169 | + | watchdog := time.AfterFunc(time.Until(deadline)+watchdogGrace, func() { | |||
| 170 | + | fired.Store(true) | |||
| 171 | + | _ = b.c.Close() | |||
| 172 | + | }) | |||
| 173 | + | defer watchdog.Stop() | |||
| 144 | // finish gives err to every call without an answer. | 174 | // finish gives err to every call without an answer. | ||
| 145 | finish := func(err error) []reply { | 175 | finish := func(err error) []reply { | ||
| 176 | + | if !errors.Is(err, ErrBroken) && (fired.Load() || !errors.Is(err, ErrTimeout)) { | |||
| 177 | + | err = fmt.Errorf("%w: %w", ErrBroken, err) | |||
| 178 | + | } | |||
| 179 | + | if errors.Is(err, ErrBroken) { | |||
| 180 | + | _ = b.c.Close() // late replies would desynchronize the next batch | |||
| 181 | + | } | |||
| 146 | for i := range out { | 182 | for i := range out { | ||
| 147 | if !answered[i] { | 183 | if !answered[i] { | ||
| 148 | out[i].err = err | 184 | out[i].err = err | ||
| @@ -152,48 +188,64 @@ func (b *conn) Do(deadline time.Time, idle time.Duration, calls []call) []reply | |||||
| 152 | } | 188 | } | ||
| 153 | for done < len(calls) { | 189 | for done < len(calls) { | ||
| 154 | for next < len(calls) && len(pending) < maxInflight { | 190 | for next < len(calls) && len(pending) < maxInflight { | ||
| 191 | + | if !time.Now().Before(deadline) { | |||
| 192 | + | return finish(errDeadline) | |||
| 193 | + | } | |||
| 155 | serial, err := b.send(calls[next]) | 194 | serial, err := b.send(calls[next]) | ||
| 156 | if err != nil { | 195 | if err != nil { | ||
| 157 | - | if errors.Is(err, zerobus.ErrClosed) { | 196 | + | return finish(err) |
| 158 | - | return finish(err) | |||
| 159 | - | } | |||
| 160 | - | out[next].err = err | |||
| 161 | - | answered[next] = true | |||
| 162 | - | done++ | |||
| 163 | - | } else { | |||
| 164 | - | pending[serial] = next | |||
| 165 | } | 197 | } | ||
| 198 | + | pending[serial] = inflight{idx: next, sent: time.Now()} | |||
| 166 | next++ | 199 | next++ | ||
| 167 | } | 200 | } | ||
| 201 | + | // Calls that waited longer than idle get no reply. | |||
| 202 | + | now := time.Now() | |||
| 203 | + | var oldest time.Time | |||
| 204 | + | for serial, p := range pending { | |||
| 205 | + | if now.Sub(p.sent) >= idle { | |||
| 206 | + | out[p.idx].err = ErrTimeout | |||
| 207 | + | answered[p.idx] = true | |||
| 208 | + | done++ | |||
| 209 | + | delete(pending, serial) | |||
| 210 | + | } else if oldest.IsZero() || p.sent.Before(oldest) { | |||
| 211 | + | oldest = p.sent | |||
| 212 | + | } | |||
| 213 | + | } | |||
| 168 | if len(pending) == 0 { | 214 | if len(pending) == 0 { | ||
| 169 | continue | 215 | continue | ||
| 170 | } | 216 | } | ||
| 171 | - | wait := min(idle, time.Until(deadline)) | 217 | + | wait := min(time.Until(oldest.Add(idle)), time.Until(deadline)) |
| 172 | if wait <= 0 { | 218 | if wait <= 0 { | ||
| 173 | - | break | 219 | + | if !time.Now().Before(deadline) { |
| 220 | + | break | |||
| 221 | + | } | |||
| 222 | + | continue | |||
| 174 | } | 223 | } | ||
| 175 | ok, err := b.readable(wait) | 224 | ok, err := b.readable(wait) | ||
| 176 | if err != nil { | 225 | if err != nil { | ||
| 177 | return finish(err) | 226 | return finish(err) | ||
| 178 | } | 227 | } | ||
| 179 | if !ok { | 228 | if !ok { | ||
| 180 | - | break | 229 | + | continue |
| 181 | } | 230 | } | ||
| 182 | m, err := b.c.ReadMessage() | 231 | m, err := b.c.ReadMessage() | ||
| 183 | if err != nil { | 232 | if err != nil { | ||
| 184 | return finish(err) | 233 | return finish(err) | ||
| 185 | } | 234 | } | ||
| 186 | - | i, found := pending[m.ReplySerial] | 235 | + | p, found := pending[m.ReplySerial] |
| 187 | if !found || (m.Type != zerobus.TypeMethodReturn && m.Type != zerobus.TypeError) { | 236 | if !found || (m.Type != zerobus.TypeMethodReturn && m.Type != zerobus.TypeError) { | ||
| 188 | // A signal, or the late reply of an earlier batch. | 237 | // A signal, or the late reply of an earlier batch. | ||
| 189 | continue | 238 | continue | ||
| 190 | } | 239 | } | ||
| 191 | delete(pending, m.ReplySerial) | 240 | delete(pending, m.ReplySerial) | ||
| 192 | - | decode(m, calls[i], &out[i]) | 241 | + | decode(m, calls[p.idx], &out[p.idx]) |
| 193 | - | answered[i] = true | 242 | + | answered[p.idx] = true |
| 194 | done++ | 243 | done++ | ||
| 195 | } | 244 | } | ||
| 196 | - | return finish(ErrTimeout) | 245 | + | if done == len(calls) { |
| 246 | + | return out | |||
| 247 | + | } | |||
| 248 | + | return finish(errDeadline) | |||
| 197 | } | 249 | } | ||
| 198 | 250 | ||||
| 199 | // send builds and sends one call and returns its serial. | 251 | // send builds and sends one call and returns its serial. | ||
M internal/adapters/hints/dump.go +39 -6
| @@ -11,6 +11,7 @@ import ( | |||
| 11 | 11 | "path/filepath" | |
| 12 | 12 | "sort" | |
| 13 | 13 | "strings" | |
| 14 | + | "syscall" | |
| 14 | 15 | "time" | |
| 15 | 16 | ||
| 16 | 17 | "github.com/bnema/neferwl/internal/ports" | |
| @@ -101,7 +102,7 @@ func ms(d time.Duration) float64 { return float64(d.Microseconds()) / 1000 } | |||
| 101 | 102 | ||
| 102 | 103 | // writeDump writes the three files of j into dir, then prunes old groups. | |
| 103 | 104 | func writeDump(dir string, j *dumpJob) error { | |
| 104 | - | if err := os.MkdirAll(dir, 0o700); err != nil { | |
| 105 | + | if err := secureDir(dir); err != nil { | |
| 105 | 106 | return err | |
| 106 | 107 | } | |
| 107 | 108 | prefix := filepath.Join(dir, j.when.Format("20060102-150405.000")+"-"+safeName(j.req.Output)) | |
| @@ -184,16 +185,48 @@ func writePNG(path string, img image.Image) error { | |||
| 184 | 185 | }) | |
| 185 | 186 | } | |
| 186 | 187 | ||
| 187 | - | func writeFile(path string, fill func(*os.File) error) error { | |
| 188 | - | f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) | |
| 188 | + | // secureDir makes sure dir is a real directory owned by the current user and | |
| 189 | + | // private to it, whatever it was before. | |
| 190 | + | func secureDir(dir string) error { | |
| 191 | + | if err := os.MkdirAll(dir, 0o700); err != nil { | |
| 192 | + | return err | |
| 193 | + | } | |
| 194 | + | fi, err := os.Lstat(dir) | |
| 189 | 195 | if err != nil { | |
| 190 | 196 | return err | |
| 191 | 197 | } | |
| 192 | - | if err := fill(f); err != nil { | |
| 193 | - | _ = f.Close() | |
| 198 | + | if !fi.IsDir() { | |
| 199 | + | return fmt.Errorf("dump dir %s is not a directory", dir) | |
| 200 | + | } | |
| 201 | + | if st, ok := fi.Sys().(*syscall.Stat_t); ok && int(st.Uid) != os.Getuid() { | |
| 202 | + | return fmt.Errorf("dump dir %s is not owned by the current user", dir) | |
| 203 | + | } | |
| 204 | + | return os.Chmod(dir, 0o700) | |
| 205 | + | } | |
| 206 | + | ||
| 207 | + | // writeFile fills a fresh private temporary file in the directory of path and | |
| 208 | + | // renames it over path: an existing file or symlink there is replaced, never | |
| 209 | + | // followed. | |
| 210 | + | func writeFile(path string, fill func(*os.File) error) error { | |
| 211 | + | f, err := os.CreateTemp(filepath.Dir(path), ".dump-*") | |
| 212 | + | if err != nil { | |
| 194 | 213 | return err | |
| 195 | 214 | } | |
| 196 | - | return f.Close() | |
| 215 | + | tmp := f.Name() | |
| 216 | + | err = f.Chmod(0o600) | |
| 217 | + | if err == nil { | |
| 218 | + | err = fill(f) | |
| 219 | + | } | |
| 220 | + | if cerr := f.Close(); err == nil { | |
| 221 | + | err = cerr | |
| 222 | + | } | |
| 223 | + | if err == nil { | |
| 224 | + | err = os.Rename(tmp, path) | |
| 225 | + | } | |
| 226 | + | if err != nil { | |
| 227 | + | _ = os.Remove(tmp) | |
| 228 | + | } | |
| 229 | + | return err | |
| 197 | 230 | } | |
| 198 | 231 | ||
| 199 | 232 | // outline draws a 2 px frame inside r, clipped to img. | |
| @@ -11,6 +11,7 @@ import ( | |||||
| 11 | "path/filepath" | 11 | "path/filepath" | ||
| 12 | "sort" | 12 | "sort" | ||
| 13 | "strings" | 13 | "strings" | ||
| 14 | + | "syscall" | |||
| 14 | "time" | 15 | "time" | ||
| 15 | 16 | ||||
| 16 | "github.com/bnema/neferwl/internal/ports" | 17 | "github.com/bnema/neferwl/internal/ports" | ||
| @@ -101,7 +102,7 @@ func ms(d time.Duration) float64 { return float64(d.Microseconds()) / 1000 } | |||||
| 101 | 102 | ||||
| 102 | // writeDump writes the three files of j into dir, then prunes old groups. | 103 | // writeDump writes the three files of j into dir, then prunes old groups. | ||
| 103 | func writeDump(dir string, j *dumpJob) error { | 104 | func writeDump(dir string, j *dumpJob) error { | ||
| 104 | - | if err := os.MkdirAll(dir, 0o700); err != nil { | 105 | + | if err := secureDir(dir); err != nil { |
| 105 | return err | 106 | return err | ||
| 106 | } | 107 | } | ||
| 107 | prefix := filepath.Join(dir, j.when.Format("20060102-150405.000")+"-"+safeName(j.req.Output)) | 108 | prefix := filepath.Join(dir, j.when.Format("20060102-150405.000")+"-"+safeName(j.req.Output)) | ||
| @@ -184,16 +185,48 @@ func writePNG(path string, img image.Image) error { | |||||
| 184 | }) | 185 | }) | ||
| 185 | } | 186 | } | ||
| 186 | 187 | ||||
| 187 | - | func writeFile(path string, fill func(*os.File) error) error { | 188 | + | // secureDir makes sure dir is a real directory owned by the current user and |
| 188 | - | f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) | 189 | + | // private to it, whatever it was before. |
| 190 | + | func secureDir(dir string) error { | |||
| 191 | + | if err := os.MkdirAll(dir, 0o700); err != nil { | |||
| 192 | + | return err | |||
| 193 | + | } | |||
| 194 | + | fi, err := os.Lstat(dir) | |||
| 189 | if err != nil { | 195 | if err != nil { | ||
| 190 | return err | 196 | return err | ||
| 191 | } | 197 | } | ||
| 192 | - | if err := fill(f); err != nil { | 198 | + | if !fi.IsDir() { |
| 193 | - | _ = f.Close() | 199 | + | return fmt.Errorf("dump dir %s is not a directory", dir) |
| 200 | + | } | |||
| 201 | + | if st, ok := fi.Sys().(*syscall.Stat_t); ok && int(st.Uid) != os.Getuid() { | |||
| 202 | + | return fmt.Errorf("dump dir %s is not owned by the current user", dir) | |||
| 203 | + | } | |||
| 204 | + | return os.Chmod(dir, 0o700) | |||
| 205 | + | } | |||
| 206 | + | ||||
| 207 | + | // writeFile fills a fresh private temporary file in the directory of path and | |||
| 208 | + | // renames it over path: an existing file or symlink there is replaced, never | |||
| 209 | + | // followed. | |||
| 210 | + | func writeFile(path string, fill func(*os.File) error) error { | |||
| 211 | + | f, err := os.CreateTemp(filepath.Dir(path), ".dump-*") | |||
| 212 | + | if err != nil { | |||
| 194 | return err | 213 | return err | ||
| 195 | } | 214 | } | ||
| 196 | - | return f.Close() | 215 | + | tmp := f.Name() |
| 216 | + | err = f.Chmod(0o600) | |||
| 217 | + | if err == nil { | |||
| 218 | + | err = fill(f) | |||
| 219 | + | } | |||
| 220 | + | if cerr := f.Close(); err == nil { | |||
| 221 | + | err = cerr | |||
| 222 | + | } | |||
| 223 | + | if err == nil { | |||
| 224 | + | err = os.Rename(tmp, path) | |||
| 225 | + | } | |||
| 226 | + | if err != nil { | |||
| 227 | + | _ = os.Remove(tmp) | |||
| 228 | + | } | |||
| 229 | + | return err | |||
| 197 | } | 230 | } | ||
| 198 | 231 | ||||
| 199 | // outline draws a 2 px frame inside r, clipped to img. | 232 | // outline draws a 2 px frame inside r, clipped to img. | ||
A internal/adapters/hints/dump_test.go +40 -0
| @@ -0,0 +1,40 @@ | |||
| 1 | + | package hints | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "os" | |
| 5 | + | "path/filepath" | |
| 6 | + | "testing" | |
| 7 | + | "time" | |
| 8 | + | ||
| 9 | + | "github.com/stretchr/testify/require" | |
| 10 | + | ||
| 11 | + | "github.com/bnema/neferwl/internal/ports" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // An existing dump directory is made private, and a symlink at a file name is | |
| 15 | + | // replaced instead of followed. | |
| 16 | + | func TestWriteDumpPrivate(t *testing.T) { | |
| 17 | + | base := t.TempDir() | |
| 18 | + | dir := filepath.Join(base, "hints") | |
| 19 | + | require.NoError(t, os.Mkdir(dir, 0o755)) | |
| 20 | + | require.NoError(t, os.Chmod(dir, 0o755)) | |
| 21 | + | victim := filepath.Join(base, "victim") | |
| 22 | + | require.NoError(t, os.WriteFile(victim, []byte("keep"), 0o644)) | |
| 23 | + | when := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) | |
| 24 | + | prefix := filepath.Join(dir, when.Format("20060102-150405.000")+"-o") | |
| 25 | + | require.NoError(t, os.Symlink(victim, prefix+".json")) | |
| 26 | + | pix := make([]byte, 4*4*4) | |
| 27 | + | j := &dumpJob{when: when, req: ports.HintScanRequest{Output: "o", Width: 4, Height: 4, Scale: 1}, pix: pix, stride: 16, m: &mapping{}} | |
| 28 | + | require.NoError(t, writeDump(dir, j)) | |
| 29 | + | ||
| 30 | + | fi, err := os.Stat(dir) | |
| 31 | + | require.NoError(t, err) | |
| 32 | + | require.Equal(t, os.FileMode(0o700), fi.Mode().Perm()) | |
| 33 | + | got, err := os.ReadFile(victim) | |
| 34 | + | require.NoError(t, err) | |
| 35 | + | require.Equal(t, "keep", string(got)) | |
| 36 | + | li, err := os.Lstat(prefix + ".json") | |
| 37 | + | require.NoError(t, err) | |
| 38 | + | require.True(t, li.Mode().IsRegular()) | |
| 39 | + | require.Equal(t, os.FileMode(0o600), li.Mode().Perm()) | |
| 40 | + | } | |
| @@ -0,0 +1,40 @@ | |||||
| 1 | + | package hints | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "os" | |||
| 5 | + | "path/filepath" | |||
| 6 | + | "testing" | |||
| 7 | + | "time" | |||
| 8 | + | ||||
| 9 | + | "github.com/stretchr/testify/require" | |||
| 10 | + | ||||
| 11 | + | "github.com/bnema/neferwl/internal/ports" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | // An existing dump directory is made private, and a symlink at a file name is | |||
| 15 | + | // replaced instead of followed. | |||
| 16 | + | func TestWriteDumpPrivate(t *testing.T) { | |||
| 17 | + | base := t.TempDir() | |||
| 18 | + | dir := filepath.Join(base, "hints") | |||
| 19 | + | require.NoError(t, os.Mkdir(dir, 0o755)) | |||
| 20 | + | require.NoError(t, os.Chmod(dir, 0o755)) | |||
| 21 | + | victim := filepath.Join(base, "victim") | |||
| 22 | + | require.NoError(t, os.WriteFile(victim, []byte("keep"), 0o644)) | |||
| 23 | + | when := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) | |||
| 24 | + | prefix := filepath.Join(dir, when.Format("20060102-150405.000")+"-o") | |||
| 25 | + | require.NoError(t, os.Symlink(victim, prefix+".json")) | |||
| 26 | + | pix := make([]byte, 4*4*4) | |||
| 27 | + | j := &dumpJob{when: when, req: ports.HintScanRequest{Output: "o", Width: 4, Height: 4, Scale: 1}, pix: pix, stride: 16, m: &mapping{}} | |||
| 28 | + | require.NoError(t, writeDump(dir, j)) | |||
| 29 | + | ||||
| 30 | + | fi, err := os.Stat(dir) | |||
| 31 | + | require.NoError(t, err) | |||
| 32 | + | require.Equal(t, os.FileMode(0o700), fi.Mode().Perm()) | |||
| 33 | + | got, err := os.ReadFile(victim) | |||
| 34 | + | require.NoError(t, err) | |||
| 35 | + | require.Equal(t, "keep", string(got)) | |||
| 36 | + | li, err := os.Lstat(prefix + ".json") | |||
| 37 | + | require.NoError(t, err) | |||
| 38 | + | require.True(t, li.Mode().IsRegular()) | |||
| 39 | + | require.Equal(t, os.FileMode(0o600), li.Mode().Perm()) | |||
| 40 | + | } | |||
M internal/adapters/hints/hints.go +11 -0
| @@ -81,9 +81,17 @@ func run(ctx context.Context, requests <-chan ports.HintScanRequest, captures ch | |||
| 81 | 81 | select { | |
| 82 | 82 | case <-ctx.Done(): | |
| 83 | 83 | return nil | |
| 84 | + | case done := <-captured: | |
| 85 | + | // A capture abandoned by a timeout is finally done: release it. | |
| 86 | + | w.reap(done.ID) | |
| 84 | 87 | case req := <-requests: | |
| 85 | 88 | res := w.serve(ctx, req, requests, captures, captured, results, timeout) | |
| 86 | 89 | if !send(ctx, results, res) { | |
| 90 | + | // The dump job will never run: release its frame. | |
| 91 | + | if j := w.pending; j != nil { | |
| 92 | + | w.pending = nil | |
| 93 | + | j.m.unmap() | |
| 94 | + | } | |
| 87 | 95 | return nil | |
| 88 | 96 | } | |
| 89 | 97 | w.dump(ctx) | |
| @@ -370,6 +378,9 @@ func (w *worker) scan(ctx context.Context, req ports.HintScanRequest, fr frame, | |||
| 370 | 378 | nextCache[j.key] = bandCache{hash: j.hash, ext: j.ext, words: j.words} | |
| 371 | 379 | for _, word := range j.words { | |
| 372 | 380 | word.Rect = word.Rect.Add(r.Min) | |
| 381 | + | if word.Underlined { | |
| 382 | + | word.UnderL, word.UnderR = word.UnderL+r.Min.X, word.UnderR+r.Min.X | |
| 383 | + | } | |
| 373 | 384 | if !duplicateWord(words, word) { | |
| 374 | 385 | words = append(words, word) | |
| 375 | 386 | } | |
| @@ -81,9 +81,17 @@ func run(ctx context.Context, requests <-chan ports.HintScanRequest, captures ch | |||||
| 81 | select { | 81 | select { | ||
| 82 | case <-ctx.Done(): | 82 | case <-ctx.Done(): | ||
| 83 | return nil | 83 | return nil | ||
| 84 | + | case done := <-captured: | |||
| 85 | + | // A capture abandoned by a timeout is finally done: release it. | |||
| 86 | + | w.reap(done.ID) | |||
| 84 | case req := <-requests: | 87 | case req := <-requests: | ||
| 85 | res := w.serve(ctx, req, requests, captures, captured, results, timeout) | 88 | res := w.serve(ctx, req, requests, captures, captured, results, timeout) | ||
| 86 | if !send(ctx, results, res) { | 89 | if !send(ctx, results, res) { | ||
| 90 | + | // The dump job will never run: release its frame. | |||
| 91 | + | if j := w.pending; j != nil { | |||
| 92 | + | w.pending = nil | |||
| 93 | + | j.m.unmap() | |||
| 94 | + | } | |||
| 87 | return nil | 95 | return nil | ||
| 88 | } | 96 | } | ||
| 89 | w.dump(ctx) | 97 | w.dump(ctx) | ||
| @@ -370,6 +378,9 @@ func (w *worker) scan(ctx context.Context, req ports.HintScanRequest, fr frame, | |||||
| 370 | nextCache[j.key] = bandCache{hash: j.hash, ext: j.ext, words: j.words} | 378 | nextCache[j.key] = bandCache{hash: j.hash, ext: j.ext, words: j.words} | ||
| 371 | for _, word := range j.words { | 379 | for _, word := range j.words { | ||
| 372 | word.Rect = word.Rect.Add(r.Min) | 380 | word.Rect = word.Rect.Add(r.Min) | ||
| 381 | + | if word.Underlined { | |||
| 382 | + | word.UnderL, word.UnderR = word.UnderL+r.Min.X, word.UnderR+r.Min.X | |||
| 383 | + | } | |||
| 373 | if !duplicateWord(words, word) { | 384 | if !duplicateWord(words, word) { | ||
| 374 | words = append(words, word) | 385 | words = append(words, word) | ||
| 375 | } | 386 | } | ||
M internal/adapters/hints/hints_test.go +13 -0
| @@ -418,3 +418,16 @@ func TestNoTesseractDumpStillCaptures(t *testing.T) { | |||
| 418 | 418 | require.NoError(t, (<-h.results).Err) | |
| 419 | 419 | waitFiles(t, dir, 3) | |
| 420 | 420 | } | |
| 421 | + | ||
| 422 | + | // A capture that completes after its scan timed out is released while the | |
| 423 | + | // worker is idle, without another scan, and its reply does not linger. | |
| 424 | + | func TestHintLateCaptureReaped(t *testing.T) { | |
| 425 | + | h := startCapturing(t, 50*time.Millisecond) | |
| 426 | + | h.requests <- ports.HintScanRequest{Seq: 1, Output: "o", Width: 64, Height: 32, Scale: 1} | |
| 427 | + | creq := <-h.captures | |
| 428 | + | res := <-h.results | |
| 429 | + | require.ErrorIs(t, res.Err, ErrTimeout) | |
| 430 | + | // The worker is idle; the output finally answers. | |
| 431 | + | h.captured <- ports.CaptureDone{ID: creq.ID, Output: creq.Output} | |
| 432 | + | require.Eventually(t, func() bool { return len(h.captured) == 0 }, time.Second, time.Millisecond) | |
| 433 | + | } | |
| @@ -418,3 +418,16 @@ func TestNoTesseractDumpStillCaptures(t *testing.T) { | |||||
| 418 | require.NoError(t, (<-h.results).Err) | 418 | require.NoError(t, (<-h.results).Err) | ||
| 419 | waitFiles(t, dir, 3) | 419 | waitFiles(t, dir, 3) | ||
| 420 | } | 420 | } | ||
| 421 | + | ||||
| 422 | + | // A capture that completes after its scan timed out is released while the | |||
| 423 | + | // worker is idle, without another scan, and its reply does not linger. | |||
| 424 | + | func TestHintLateCaptureReaped(t *testing.T) { | |||
| 425 | + | h := startCapturing(t, 50*time.Millisecond) | |||
| 426 | + | h.requests <- ports.HintScanRequest{Seq: 1, Output: "o", Width: 64, Height: 32, Scale: 1} | |||
| 427 | + | creq := <-h.captures | |||
| 428 | + | res := <-h.results | |||
| 429 | + | require.ErrorIs(t, res.Err, ErrTimeout) | |||
| 430 | + | // The worker is idle; the output finally answers. | |||
| 431 | + | h.captured <- ports.CaptureDone{ID: creq.ID, Output: creq.Output} | |||
| 432 | + | require.Eventually(t, func() bool { return len(h.captured) == 0 }, time.Second, time.Millisecond) | |||
| 433 | + | } | |||
M internal/adapters/hints/ocr.go +23 -2
| @@ -30,6 +30,12 @@ type OCRWord struct { | |||
| 30 | 30 | Text string `json:"text"` | |
| 31 | 31 | Rect image.Rectangle `json:"rect"` | |
| 32 | 32 | Confidence float64 `json:"confidence"` | |
| 33 | + | // Underlined is set when a thin line runs under the word. UnderL and | |
| 34 | + | // UnderR are the x extent of that line (crop-local), which can reach the | |
| 35 | + | // neighbouring words of one underlined link. | |
| 36 | + | Underlined bool `json:"underlined,omitempty"` | |
| 37 | + | UnderL int `json:"under_l,omitempty"` | |
| 38 | + | UnderR int `json:"under_r,omitempty"` | |
| 33 | 39 | } | |
| 34 | 40 | ||
| 35 | 41 | var languageWarning sync.Once | |
| @@ -154,12 +160,24 @@ func bandsOf(height int) []band { | |||
| 154 | 160 | func (w *worker) recognize(ctx context.Context, jobs []*bandJob) { | |
| 155 | 161 | ctx, cancel := context.WithTimeout(ctx, 15*time.Second) | |
| 156 | 162 | defer cancel() | |
| 163 | + | // One scan-wide pool: at most ocrWorkers tesseract processes run at once, | |
| 164 | + | // whatever the number of windows. | |
| 165 | + | sem := make(chan struct{}, max(ocrWorkers, 1)) | |
| 157 | 166 | var wg sync.WaitGroup | |
| 158 | 167 | for _, j := range jobs { | |
| 159 | 168 | if j.hit { | |
| 160 | 169 | continue | |
| 161 | 170 | } | |
| 162 | - | wg.Go(func() { j.words, j.err = readBand(ctx, &w.ocr, j) }) | |
| 171 | + | select { | |
| 172 | + | case sem <- struct{}{}: | |
| 173 | + | case <-ctx.Done(): | |
| 174 | + | j.err = ctx.Err() | |
| 175 | + | continue | |
| 176 | + | } | |
| 177 | + | wg.Go(func() { | |
| 178 | + | defer func() { <-sem }() | |
| 179 | + | j.words, j.err = readBand(ctx, &w.ocr, j) | |
| 180 | + | }) | |
| 163 | 181 | } | |
| 164 | 182 | wg.Wait() | |
| 165 | 183 | } | |
| @@ -187,6 +205,7 @@ func readBand(ctx context.Context, st *ocrState, j *bandJob) ([]OCRWord, error) | |||
| 187 | 205 | words[i].Rect = image.Rect(r.Min.X/2, r.Min.Y/2, (r.Max.X+1)/2, (r.Max.Y+1)/2) | |
| 188 | 206 | } | |
| 189 | 207 | } | |
| 208 | + | markUnderlined(j.gray, words) | |
| 190 | 209 | owned := words[:0] | |
| 191 | 210 | for _, word := range words { | |
| 192 | 211 | // Overlap supplies context, but each word belongs to exactly one band. | |
| @@ -273,6 +292,7 @@ func textLinks(words []OCRWord, scale float64) []ports.HintTarget { | |||
| 273 | 292 | return words[i].Rect.Min.X < words[j].Rect.Min.X | |
| 274 | 293 | }) | |
| 275 | 294 | var out []ports.HintTarget | |
| 295 | + | var urls []image.Rectangle // boxes of the URL targets, in pixels | |
| 276 | 296 | used := make([]bool, len(words)) | |
| 277 | 297 | for i, word := range words { | |
| 278 | 298 | if used[i] { | |
| @@ -310,8 +330,9 @@ func textLinks(words []OCRWord, scale float64) []ports.HintTarget { | |||
| 310 | 330 | x, y := int(math.Floor(float64(rect.Min.X)/scale)), int(math.Floor(float64(rect.Min.Y)/scale)) | |
| 311 | 331 | right, bottom := int(math.Ceil(float64(rect.Max.X)/scale)), int(math.Ceil(float64(rect.Max.Y)/scale)) | |
| 312 | 332 | out = append(out, ports.HintTarget{Kind: ports.HintLink, Text: true, Rect: ports.Rect{X: x, Y: y, W: right - x, H: bottom - y}}) | |
| 333 | + | urls = append(urls, rect) | |
| 313 | 334 | } | |
| 314 | - | return out | |
| 335 | + | return append(out, underlinedLinks(words, urls, scale)...) | |
| 315 | 336 | } | |
| 316 | 337 | ||
| 317 | 338 | func abs(x int) int { | |
| @@ -30,6 +30,12 @@ type OCRWord struct { | |||||
| 30 | Text string `json:"text"` | 30 | Text string `json:"text"` | ||
| 31 | Rect image.Rectangle `json:"rect"` | 31 | Rect image.Rectangle `json:"rect"` | ||
| 32 | Confidence float64 `json:"confidence"` | 32 | Confidence float64 `json:"confidence"` | ||
| 33 | + | // Underlined is set when a thin line runs under the word. UnderL and | |||
| 34 | + | // UnderR are the x extent of that line (crop-local), which can reach the | |||
| 35 | + | // neighbouring words of one underlined link. | |||
| 36 | + | Underlined bool `json:"underlined,omitempty"` | |||
| 37 | + | UnderL int `json:"under_l,omitempty"` | |||
| 38 | + | UnderR int `json:"under_r,omitempty"` | |||
| 33 | } | 39 | } | ||
| 34 | 40 | ||||
| 35 | var languageWarning sync.Once | 41 | var languageWarning sync.Once | ||
| @@ -154,12 +160,24 @@ func bandsOf(height int) []band { | |||||
| 154 | func (w *worker) recognize(ctx context.Context, jobs []*bandJob) { | 160 | func (w *worker) recognize(ctx context.Context, jobs []*bandJob) { | ||
| 155 | ctx, cancel := context.WithTimeout(ctx, 15*time.Second) | 161 | ctx, cancel := context.WithTimeout(ctx, 15*time.Second) | ||
| 156 | defer cancel() | 162 | defer cancel() | ||
| 163 | + | // One scan-wide pool: at most ocrWorkers tesseract processes run at once, | |||
| 164 | + | // whatever the number of windows. | |||
| 165 | + | sem := make(chan struct{}, max(ocrWorkers, 1)) | |||
| 157 | var wg sync.WaitGroup | 166 | var wg sync.WaitGroup | ||
| 158 | for _, j := range jobs { | 167 | for _, j := range jobs { | ||
| 159 | if j.hit { | 168 | if j.hit { | ||
| 160 | continue | 169 | continue | ||
| 161 | } | 170 | } | ||
| 162 | - | wg.Go(func() { j.words, j.err = readBand(ctx, &w.ocr, j) }) | 171 | + | select { |
| 172 | + | case sem <- struct{}{}: | |||
| 173 | + | case <-ctx.Done(): | |||
| 174 | + | j.err = ctx.Err() | |||
| 175 | + | continue | |||
| 176 | + | } | |||
| 177 | + | wg.Go(func() { | |||
| 178 | + | defer func() { <-sem }() | |||
| 179 | + | j.words, j.err = readBand(ctx, &w.ocr, j) | |||
| 180 | + | }) | |||
| 163 | } | 181 | } | ||
| 164 | wg.Wait() | 182 | wg.Wait() | ||
| 165 | } | 183 | } | ||
| @@ -187,6 +205,7 @@ func readBand(ctx context.Context, st *ocrState, j *bandJob) ([]OCRWord, error) | |||||
| 187 | words[i].Rect = image.Rect(r.Min.X/2, r.Min.Y/2, (r.Max.X+1)/2, (r.Max.Y+1)/2) | 205 | words[i].Rect = image.Rect(r.Min.X/2, r.Min.Y/2, (r.Max.X+1)/2, (r.Max.Y+1)/2) | ||
| 188 | } | 206 | } | ||
| 189 | } | 207 | } | ||
| 208 | + | markUnderlined(j.gray, words) | |||
| 190 | owned := words[:0] | 209 | owned := words[:0] | ||
| 191 | for _, word := range words { | 210 | for _, word := range words { | ||
| 192 | // Overlap supplies context, but each word belongs to exactly one band. | 211 | // Overlap supplies context, but each word belongs to exactly one band. | ||
| @@ -273,6 +292,7 @@ func textLinks(words []OCRWord, scale float64) []ports.HintTarget { | |||||
| 273 | return words[i].Rect.Min.X < words[j].Rect.Min.X | 292 | return words[i].Rect.Min.X < words[j].Rect.Min.X | ||
| 274 | }) | 293 | }) | ||
| 275 | var out []ports.HintTarget | 294 | var out []ports.HintTarget | ||
| 295 | + | var urls []image.Rectangle // boxes of the URL targets, in pixels | |||
| 276 | used := make([]bool, len(words)) | 296 | used := make([]bool, len(words)) | ||
| 277 | for i, word := range words { | 297 | for i, word := range words { | ||
| 278 | if used[i] { | 298 | if used[i] { | ||
| @@ -310,8 +330,9 @@ func textLinks(words []OCRWord, scale float64) []ports.HintTarget { | |||||
| 310 | x, y := int(math.Floor(float64(rect.Min.X)/scale)), int(math.Floor(float64(rect.Min.Y)/scale)) | 330 | x, y := int(math.Floor(float64(rect.Min.X)/scale)), int(math.Floor(float64(rect.Min.Y)/scale)) | ||
| 311 | right, bottom := int(math.Ceil(float64(rect.Max.X)/scale)), int(math.Ceil(float64(rect.Max.Y)/scale)) | 331 | right, bottom := int(math.Ceil(float64(rect.Max.X)/scale)), int(math.Ceil(float64(rect.Max.Y)/scale)) | ||
| 312 | out = append(out, ports.HintTarget{Kind: ports.HintLink, Text: true, Rect: ports.Rect{X: x, Y: y, W: right - x, H: bottom - y}}) | 332 | out = append(out, ports.HintTarget{Kind: ports.HintLink, Text: true, Rect: ports.Rect{X: x, Y: y, W: right - x, H: bottom - y}}) | ||
| 333 | + | urls = append(urls, rect) | |||
| 313 | } | 334 | } | ||
| 314 | - | return out | 335 | + | return append(out, underlinedLinks(words, urls, scale)...) |
| 315 | } | 336 | } | ||
| 316 | 337 | ||||
| 317 | func abs(x int) int { | 338 | func abs(x int) int { | ||
M internal/adapters/hints/ocr_test.go +45 -0
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "io" | |
| 9 | 9 | "os" | |
| 10 | 10 | "os/exec" | |
| 11 | + | "path/filepath" | |
| 11 | 12 | "reflect" | |
| 12 | 13 | "strings" | |
| 13 | 14 | "testing" | |
| @@ -191,3 +192,47 @@ func TestPGMMatchesPNG(t *testing.T) { | |||
| 191 | 192 | t.Fatalf("PNG %d words, PGM %d words", len(res[0]), len(res[1])) | |
| 192 | 193 | } | |
| 193 | 194 | } | |
| 195 | + | ||
| 196 | + | // TestRecognizeCapsProcesses runs the bands of several windows through a | |
| 197 | + | // stand-in tesseract that logs when each process starts and ends. | |
| 198 | + | func TestRecognizeCapsProcesses(t *testing.T) { | |
| 199 | + | dir := t.TempDir() | |
| 200 | + | logPath := filepath.Join(dir, "log") | |
| 201 | + | script := filepath.Join(dir, "tesseract") | |
| 202 | + | body := "#!/bin/sh\ncat >/dev/null\necho s >> " + logPath + "\nsleep 0.1\necho e >> " + logPath + "\nprintf 'level\\tpage_num\\tblock_num\\tpar_num\\tline_num\\tword_num\\tleft\\ttop\\twidth\\theight\\tconf\\ttext\\n'\n" | |
| 203 | + | if err := os.WriteFile(script, []byte(body), 0o700); err != nil { | |
| 204 | + | t.Fatal(err) | |
| 205 | + | } | |
| 206 | + | old := ocrWorkers | |
| 207 | + | ocrWorkers = 2 | |
| 208 | + | t.Cleanup(func() { ocrWorkers = old }) | |
| 209 | + | w := &worker{ocr: ocrState{done: true, binary: script, lang: "eng"}} | |
| 210 | + | var jobs []*bandJob | |
| 211 | + | for range 8 { // several windows' bands | |
| 212 | + | g := image.NewGray(image.Rect(0, 0, 8, 8)) | |
| 213 | + | jobs = append(jobs, &bandJob{ext: g.Rect, own: [2]int{0, 8}, gray: g}) | |
| 214 | + | } | |
| 215 | + | w.recognize(context.Background(), jobs) | |
| 216 | + | data, err := os.ReadFile(logPath) | |
| 217 | + | if err != nil { | |
| 218 | + | t.Fatal(err) | |
| 219 | + | } | |
| 220 | + | cur, peak, starts := 0, 0, 0 | |
| 221 | + | for _, l := range strings.Fields(string(data)) { | |
| 222 | + | if l == "s" { | |
| 223 | + | cur++ | |
| 224 | + | starts++ | |
| 225 | + | peak = max(peak, cur) | |
| 226 | + | } else { | |
| 227 | + | cur-- | |
| 228 | + | } | |
| 229 | + | } | |
| 230 | + | if starts != 8 || peak > 2 { | |
| 231 | + | t.Fatalf("starts=%d peak=%d", starts, peak) | |
| 232 | + | } | |
| 233 | + | for _, j := range jobs { | |
| 234 | + | if j.err != nil { | |
| 235 | + | t.Fatal(j.err) | |
| 236 | + | } | |
| 237 | + | } | |
| 238 | + | } | |
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "io" | 8 | "io" | ||
| 9 | "os" | 9 | "os" | ||
| 10 | "os/exec" | 10 | "os/exec" | ||
| 11 | + | "path/filepath" | |||
| 11 | "reflect" | 12 | "reflect" | ||
| 12 | "strings" | 13 | "strings" | ||
| 13 | "testing" | 14 | "testing" | ||
| @@ -191,3 +192,47 @@ func TestPGMMatchesPNG(t *testing.T) { | |||||
| 191 | t.Fatalf("PNG %d words, PGM %d words", len(res[0]), len(res[1])) | 192 | t.Fatalf("PNG %d words, PGM %d words", len(res[0]), len(res[1])) | ||
| 192 | } | 193 | } | ||
| 193 | } | 194 | } | ||
| 195 | + | ||||
| 196 | + | // TestRecognizeCapsProcesses runs the bands of several windows through a | |||
| 197 | + | // stand-in tesseract that logs when each process starts and ends. | |||
| 198 | + | func TestRecognizeCapsProcesses(t *testing.T) { | |||
| 199 | + | dir := t.TempDir() | |||
| 200 | + | logPath := filepath.Join(dir, "log") | |||
| 201 | + | script := filepath.Join(dir, "tesseract") | |||
| 202 | + | body := "#!/bin/sh\ncat >/dev/null\necho s >> " + logPath + "\nsleep 0.1\necho e >> " + logPath + "\nprintf 'level\\tpage_num\\tblock_num\\tpar_num\\tline_num\\tword_num\\tleft\\ttop\\twidth\\theight\\tconf\\ttext\\n'\n" | |||
| 203 | + | if err := os.WriteFile(script, []byte(body), 0o700); err != nil { | |||
| 204 | + | t.Fatal(err) | |||
| 205 | + | } | |||
| 206 | + | old := ocrWorkers | |||
| 207 | + | ocrWorkers = 2 | |||
| 208 | + | t.Cleanup(func() { ocrWorkers = old }) | |||
| 209 | + | w := &worker{ocr: ocrState{done: true, binary: script, lang: "eng"}} | |||
| 210 | + | var jobs []*bandJob | |||
| 211 | + | for range 8 { // several windows' bands | |||
| 212 | + | g := image.NewGray(image.Rect(0, 0, 8, 8)) | |||
| 213 | + | jobs = append(jobs, &bandJob{ext: g.Rect, own: [2]int{0, 8}, gray: g}) | |||
| 214 | + | } | |||
| 215 | + | w.recognize(context.Background(), jobs) | |||
| 216 | + | data, err := os.ReadFile(logPath) | |||
| 217 | + | if err != nil { | |||
| 218 | + | t.Fatal(err) | |||
| 219 | + | } | |||
| 220 | + | cur, peak, starts := 0, 0, 0 | |||
| 221 | + | for _, l := range strings.Fields(string(data)) { | |||
| 222 | + | if l == "s" { | |||
| 223 | + | cur++ | |||
| 224 | + | starts++ | |||
| 225 | + | peak = max(peak, cur) | |||
| 226 | + | } else { | |||
| 227 | + | cur-- | |||
| 228 | + | } | |||
| 229 | + | } | |||
| 230 | + | if starts != 8 || peak > 2 { | |||
| 231 | + | t.Fatalf("starts=%d peak=%d", starts, peak) | |||
| 232 | + | } | |||
| 233 | + | for _, j := range jobs { | |||
| 234 | + | if j.err != nil { | |||
| 235 | + | t.Fatal(j.err) | |||
| 236 | + | } | |||
| 237 | + | } | |||
| 238 | + | } | |||
A internal/adapters/hints/underline.go +246 -0
| @@ -0,0 +1,246 @@ | |||
| 1 | + | package hints | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "image" | |
| 5 | + | "math" | |
| 6 | + | "sort" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/neferwl/internal/ports" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | // underlineCandidate is a word with a thin line under it. | |
| 12 | + | type underlineCandidate struct { | |
| 13 | + | word int | |
| 14 | + | row int // the line's row | |
| 15 | + | from, to int // x extent of the continuous line through the word | |
| 16 | + | } | |
| 17 | + | ||
| 18 | + | // markUnderlined sets Underlined, UnderL and UnderR on the words of a band | |
| 19 | + | // that have a thin line under them. words are in the coordinates of g. | |
| 20 | + | // Lines that run on far beyond the word (rules, table borders, input boxes) | |
| 21 | + | // do not count, unless the extension is made of underlined words too. | |
| 22 | + | func markUnderlined(src *image.Gray, words []OCRWord) { | |
| 23 | + | // Words are relative to the band's own origin, whatever the image is a | |
| 24 | + | // view of. | |
| 25 | + | b := src.Bounds() | |
| 26 | + | g := &image.Gray{Pix: src.Pix[src.PixOffset(b.Min.X, b.Min.Y):], Stride: src.Stride, Rect: image.Rect(0, 0, b.Dx(), b.Dy())} | |
| 27 | + | var cands []underlineCandidate | |
| 28 | + | for i, w := range words { | |
| 29 | + | // A single glyph, or a box narrower than tall, has a flat bottom | |
| 30 | + | // stroke (8, L, 2) that is not an underline. | |
| 31 | + | if len([]rune(w.Text)) < 2 || w.Rect.Dx() < w.Rect.Dy() { | |
| 32 | + | continue | |
| 33 | + | } | |
| 34 | + | if c, ok := underlineOf(g, w.Rect); ok { | |
| 35 | + | c.word = i | |
| 36 | + | cands = append(cands, c) | |
| 37 | + | } | |
| 38 | + | } | |
| 39 | + | for _, c := range cands { | |
| 40 | + | w := words[c.word] | |
| 41 | + | h := w.Rect.Dy() | |
| 42 | + | if (w.Rect.Min.X-c.from > h && !covered(words, cands, c, -1)) || | |
| 43 | + | (c.to-w.Rect.Max.X > h && !covered(words, cands, c, 1)) { | |
| 44 | + | continue | |
| 45 | + | } | |
| 46 | + | words[c.word].Underlined, words[c.word].UnderL, words[c.word].UnderR = true, c.from, c.to | |
| 47 | + | } | |
| 48 | + | } | |
| 49 | + | ||
| 50 | + | // covered reports whether the line of c, on the given side of its word, is | |
| 51 | + | // explained by underlined words on the same line up to within a glyph of its | |
| 52 | + | // end. | |
| 53 | + | func covered(words []OCRWord, cands []underlineCandidate, c underlineCandidate, dir int) bool { | |
| 54 | + | w := words[c.word].Rect | |
| 55 | + | h := w.Dy() | |
| 56 | + | cur := w.Min.X | |
| 57 | + | if dir > 0 { | |
| 58 | + | cur = w.Max.X | |
| 59 | + | } | |
| 60 | + | for { | |
| 61 | + | if (dir < 0 && cur-c.from <= h) || (dir > 0 && c.to-cur <= h) { | |
| 62 | + | return true | |
| 63 | + | } | |
| 64 | + | next := -1 | |
| 65 | + | for _, o := range cands { | |
| 66 | + | r := words[o.word].Rect | |
| 67 | + | if abs(r.Max.Y-w.Max.Y) > max(3, h/3) { | |
| 68 | + | continue | |
| 69 | + | } | |
| 70 | + | var gap, reach int | |
| 71 | + | if dir < 0 { | |
| 72 | + | gap, reach = cur-r.Max.X, r.Min.X | |
| 73 | + | } else { | |
| 74 | + | gap, reach = r.Min.X-cur, r.Max.X | |
| 75 | + | } | |
| 76 | + | if gap < -2 || gap > h { | |
| 77 | + | continue | |
| 78 | + | } | |
| 79 | + | if next < 0 || (dir < 0 && reach < next) || (dir > 0 && reach > next) { | |
| 80 | + | next = reach | |
| 81 | + | } | |
| 82 | + | } | |
| 83 | + | if next < 0 || (dir < 0 && next >= cur) || (dir > 0 && next <= cur) { | |
| 84 | + | return false | |
| 85 | + | } | |
| 86 | + | cur = next | |
| 87 | + | } | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | // underlineOf looks for an underline under the word box r in g. | |
| 91 | + | func underlineOf(g *image.Gray, r image.Rectangle) (underlineCandidate, bool) { | |
| 92 | + | b := g.Bounds() | |
| 93 | + | box := r.Intersect(b) | |
| 94 | + | h := r.Dy() | |
| 95 | + | if box.Dx() < 4 || h < 4 { | |
| 96 | + | return underlineCandidate{}, false | |
| 97 | + | } | |
| 98 | + | bg, ink := contrastOf(g, box) | |
| 99 | + | thr := min(40, max(20, ink/2)) | |
| 100 | + | if ink < thr+10 { | |
| 101 | + | return underlineCandidate{}, false | |
| 102 | + | } | |
| 103 | + | top := max(b.Min.Y, r.Max.Y-(h*15+99)/100) | |
| 104 | + | bottom := min(b.Max.Y, r.Max.Y+(h*35+99)/100+1) | |
| 105 | + | maxThick := max(2, h/6) | |
| 106 | + | rowOK := func(y int) bool { | |
| 107 | + | row := g.Pix[(y-g.Rect.Min.Y)*g.Stride:] | |
| 108 | + | n := 0 | |
| 109 | + | for x := box.Min.X; x < box.Max.X; x++ { | |
| 110 | + | if abs(int(row[x-g.Rect.Min.X])-bg) >= thr { | |
| 111 | + | n++ | |
| 112 | + | } | |
| 113 | + | } | |
| 114 | + | return n*100 >= box.Dx()*85 | |
| 115 | + | } | |
| 116 | + | thick, first := 0, 0 | |
| 117 | + | for y := top; y <= bottom; y++ { | |
| 118 | + | if y < bottom && rowOK(y) { | |
| 119 | + | if thick == 0 { | |
| 120 | + | first = y | |
| 121 | + | } | |
| 122 | + | thick++ | |
| 123 | + | continue | |
| 124 | + | } | |
| 125 | + | if thick > 0 { | |
| 126 | + | if thick > maxThick { | |
| 127 | + | return underlineCandidate{}, false // a block, not a line | |
| 128 | + | } | |
| 129 | + | row := first + thick/2 | |
| 130 | + | from, to := box.Min.X, box.Max.X | |
| 131 | + | line := g.Pix[(row-g.Rect.Min.Y)*g.Stride:] | |
| 132 | + | on := func(x int) bool { return abs(int(line[x-g.Rect.Min.X])-bg) >= thr } | |
| 133 | + | for from > b.Min.X && on(from-1) { | |
| 134 | + | from-- | |
| 135 | + | } | |
| 136 | + | for to < b.Max.X && on(to) { | |
| 137 | + | to++ | |
| 138 | + | } | |
| 139 | + | return underlineCandidate{row: row, from: from, to: to}, true | |
| 140 | + | } | |
| 141 | + | } | |
| 142 | + | return underlineCandidate{}, false | |
| 143 | + | } | |
| 144 | + | ||
| 145 | + | // contrastOf returns the background level of box (the most common level, | |
| 146 | + | // found in bins of 16 so antialiasing does not split it, then refined to the | |
| 147 | + | // mean of that bin) and the contrast of its ink (the deviation reached by | |
| 148 | + | // the strongest 3% of pixels). | |
| 149 | + | func contrastOf(g *image.Gray, box image.Rectangle) (bg, ink int) { | |
| 150 | + | var hist [256]int | |
| 151 | + | for y := box.Min.Y; y < box.Max.Y; y++ { | |
| 152 | + | for _, v := range g.Pix[(y-g.Rect.Min.Y)*g.Stride+box.Min.X-g.Rect.Min.X:][:box.Dx()] { | |
| 153 | + | hist[v]++ | |
| 154 | + | } | |
| 155 | + | } | |
| 156 | + | total := box.Dx() * box.Dy() | |
| 157 | + | var bins [16]int | |
| 158 | + | for v, c := range hist { | |
| 159 | + | bins[v>>4] += c | |
| 160 | + | } | |
| 161 | + | best := 0 | |
| 162 | + | for i, c := range bins { | |
| 163 | + | if c > bins[best] { | |
| 164 | + | best = i | |
| 165 | + | } | |
| 166 | + | } | |
| 167 | + | sum, n := 0, 0 | |
| 168 | + | for v := best << 4; v < (best+1)<<4; v++ { | |
| 169 | + | sum += v * hist[v] | |
| 170 | + | n += hist[v] | |
| 171 | + | } | |
| 172 | + | bg = sum / max(n, 1) | |
| 173 | + | var dev [256]int | |
| 174 | + | for v, c := range hist { | |
| 175 | + | dev[abs(v-bg)] += c | |
| 176 | + | } | |
| 177 | + | n = 0 | |
| 178 | + | for d := 255; d >= 0; d-- { | |
| 179 | + | n += dev[d] | |
| 180 | + | if n*100 >= total*3 { | |
| 181 | + | return bg, d | |
| 182 | + | } | |
| 183 | + | } | |
| 184 | + | return bg, 0 | |
| 185 | + | } | |
| 186 | + | ||
| 187 | + | // underlinedLinks returns the targets for underlined words, joining the | |
| 188 | + | // words of one continuous underline. A group that overlaps a URL target (in | |
| 189 | + | // pixels) is left to it. Coordinates are scaled to logical. | |
| 190 | + | func underlinedLinks(words []OCRWord, urls []image.Rectangle, scale float64) []ports.HintTarget { | |
| 191 | + | var us []OCRWord | |
| 192 | + | for _, w := range words { | |
| 193 | + | if w.Underlined { | |
| 194 | + | us = append(us, w) | |
| 195 | + | } | |
| 196 | + | } | |
| 197 | + | sort.Slice(us, func(i, j int) bool { return us[i].Rect.Min.X < us[j].Rect.Min.X }) | |
| 198 | + | used := make([]bool, len(us)) | |
| 199 | + | var out []ports.HintTarget | |
| 200 | + | for i := range us { | |
| 201 | + | if used[i] { | |
| 202 | + | continue | |
| 203 | + | } | |
| 204 | + | used[i] = true | |
| 205 | + | rect, last := us[i].Rect, us[i] | |
| 206 | + | chars, width := len(last.Text), last.Rect.Dx() | |
| 207 | + | for { | |
| 208 | + | next := -1 | |
| 209 | + | for j := range us { | |
| 210 | + | c := us[j] | |
| 211 | + | if used[j] || c.Rect.Min.X < rect.Max.X-2 || abs(c.Rect.Max.Y-last.Rect.Max.Y) > max(3, rect.Dy()/3) { | |
| 212 | + | continue | |
| 213 | + | } | |
| 214 | + | avg := float64(width+c.Rect.Dx()) / float64(max(1, chars+len(c.Text))) | |
| 215 | + | gap := c.Rect.Min.X - rect.Max.X | |
| 216 | + | if float64(gap) > 1.2*avg || c.Rect.Min.X > last.UnderR+2 { | |
| 217 | + | continue | |
| 218 | + | } | |
| 219 | + | if next < 0 || c.Rect.Min.X < us[next].Rect.Min.X { | |
| 220 | + | next = j | |
| 221 | + | } | |
| 222 | + | } | |
| 223 | + | if next < 0 { | |
| 224 | + | break | |
| 225 | + | } | |
| 226 | + | used[next] = true | |
| 227 | + | last = us[next] | |
| 228 | + | rect = rect.Union(last.Rect) | |
| 229 | + | chars += len(last.Text) | |
| 230 | + | width += last.Rect.Dx() | |
| 231 | + | } | |
| 232 | + | dup := false | |
| 233 | + | for _, u := range urls { | |
| 234 | + | if overlap(rect, u) > .5 { | |
| 235 | + | dup = true | |
| 236 | + | } | |
| 237 | + | } | |
| 238 | + | if dup { | |
| 239 | + | continue | |
| 240 | + | } | |
| 241 | + | x, y := int(math.Floor(float64(rect.Min.X)/scale)), int(math.Floor(float64(rect.Min.Y)/scale)) | |
| 242 | + | right, bottom := int(math.Ceil(float64(rect.Max.X)/scale)), int(math.Ceil(float64(rect.Max.Y)/scale)) | |
| 243 | + | out = append(out, ports.HintTarget{Kind: ports.HintLink, Rect: ports.Rect{X: x, Y: y, W: right - x, H: bottom - y}}) | |
| 244 | + | } | |
| 245 | + | return out | |
| 246 | + | } | |
| @@ -0,0 +1,246 @@ | |||||
| 1 | + | package hints | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "image" | |||
| 5 | + | "math" | |||
| 6 | + | "sort" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/neferwl/internal/ports" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | // underlineCandidate is a word with a thin line under it. | |||
| 12 | + | type underlineCandidate struct { | |||
| 13 | + | word int | |||
| 14 | + | row int // the line's row | |||
| 15 | + | from, to int // x extent of the continuous line through the word | |||
| 16 | + | } | |||
| 17 | + | ||||
| 18 | + | // markUnderlined sets Underlined, UnderL and UnderR on the words of a band | |||
| 19 | + | // that have a thin line under them. words are in the coordinates of g. | |||
| 20 | + | // Lines that run on far beyond the word (rules, table borders, input boxes) | |||
| 21 | + | // do not count, unless the extension is made of underlined words too. | |||
| 22 | + | func markUnderlined(src *image.Gray, words []OCRWord) { | |||
| 23 | + | // Words are relative to the band's own origin, whatever the image is a | |||
| 24 | + | // view of. | |||
| 25 | + | b := src.Bounds() | |||
| 26 | + | g := &image.Gray{Pix: src.Pix[src.PixOffset(b.Min.X, b.Min.Y):], Stride: src.Stride, Rect: image.Rect(0, 0, b.Dx(), b.Dy())} | |||
| 27 | + | var cands []underlineCandidate | |||
| 28 | + | for i, w := range words { | |||
| 29 | + | // A single glyph, or a box narrower than tall, has a flat bottom | |||
| 30 | + | // stroke (8, L, 2) that is not an underline. | |||
| 31 | + | if len([]rune(w.Text)) < 2 || w.Rect.Dx() < w.Rect.Dy() { | |||
| 32 | + | continue | |||
| 33 | + | } | |||
| 34 | + | if c, ok := underlineOf(g, w.Rect); ok { | |||
| 35 | + | c.word = i | |||
| 36 | + | cands = append(cands, c) | |||
| 37 | + | } | |||
| 38 | + | } | |||
| 39 | + | for _, c := range cands { | |||
| 40 | + | w := words[c.word] | |||
| 41 | + | h := w.Rect.Dy() | |||
| 42 | + | if (w.Rect.Min.X-c.from > h && !covered(words, cands, c, -1)) || | |||
| 43 | + | (c.to-w.Rect.Max.X > h && !covered(words, cands, c, 1)) { | |||
| 44 | + | continue | |||
| 45 | + | } | |||
| 46 | + | words[c.word].Underlined, words[c.word].UnderL, words[c.word].UnderR = true, c.from, c.to | |||
| 47 | + | } | |||
| 48 | + | } | |||
| 49 | + | ||||
| 50 | + | // covered reports whether the line of c, on the given side of its word, is | |||
| 51 | + | // explained by underlined words on the same line up to within a glyph of its | |||
| 52 | + | // end. | |||
| 53 | + | func covered(words []OCRWord, cands []underlineCandidate, c underlineCandidate, dir int) bool { | |||
| 54 | + | w := words[c.word].Rect | |||
| 55 | + | h := w.Dy() | |||
| 56 | + | cur := w.Min.X | |||
| 57 | + | if dir > 0 { | |||
| 58 | + | cur = w.Max.X | |||
| 59 | + | } | |||
| 60 | + | for { | |||
| 61 | + | if (dir < 0 && cur-c.from <= h) || (dir > 0 && c.to-cur <= h) { | |||
| 62 | + | return true | |||
| 63 | + | } | |||
| 64 | + | next := -1 | |||
| 65 | + | for _, o := range cands { | |||
| 66 | + | r := words[o.word].Rect | |||
| 67 | + | if abs(r.Max.Y-w.Max.Y) > max(3, h/3) { | |||
| 68 | + | continue | |||
| 69 | + | } | |||
| 70 | + | var gap, reach int | |||
| 71 | + | if dir < 0 { | |||
| 72 | + | gap, reach = cur-r.Max.X, r.Min.X | |||
| 73 | + | } else { | |||
| 74 | + | gap, reach = r.Min.X-cur, r.Max.X | |||
| 75 | + | } | |||
| 76 | + | if gap < -2 || gap > h { | |||
| 77 | + | continue | |||
| 78 | + | } | |||
| 79 | + | if next < 0 || (dir < 0 && reach < next) || (dir > 0 && reach > next) { | |||
| 80 | + | next = reach | |||
| 81 | + | } | |||
| 82 | + | } | |||
| 83 | + | if next < 0 || (dir < 0 && next >= cur) || (dir > 0 && next <= cur) { | |||
| 84 | + | return false | |||
| 85 | + | } | |||
| 86 | + | cur = next | |||
| 87 | + | } | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | // underlineOf looks for an underline under the word box r in g. | |||
| 91 | + | func underlineOf(g *image.Gray, r image.Rectangle) (underlineCandidate, bool) { | |||
| 92 | + | b := g.Bounds() | |||
| 93 | + | box := r.Intersect(b) | |||
| 94 | + | h := r.Dy() | |||
| 95 | + | if box.Dx() < 4 || h < 4 { | |||
| 96 | + | return underlineCandidate{}, false | |||
| 97 | + | } | |||
| 98 | + | bg, ink := contrastOf(g, box) | |||
| 99 | + | thr := min(40, max(20, ink/2)) | |||
| 100 | + | if ink < thr+10 { | |||
| 101 | + | return underlineCandidate{}, false | |||
| 102 | + | } | |||
| 103 | + | top := max(b.Min.Y, r.Max.Y-(h*15+99)/100) | |||
| 104 | + | bottom := min(b.Max.Y, r.Max.Y+(h*35+99)/100+1) | |||
| 105 | + | maxThick := max(2, h/6) | |||
| 106 | + | rowOK := func(y int) bool { | |||
| 107 | + | row := g.Pix[(y-g.Rect.Min.Y)*g.Stride:] | |||
| 108 | + | n := 0 | |||
| 109 | + | for x := box.Min.X; x < box.Max.X; x++ { | |||
| 110 | + | if abs(int(row[x-g.Rect.Min.X])-bg) >= thr { | |||
| 111 | + | n++ | |||
| 112 | + | } | |||
| 113 | + | } | |||
| 114 | + | return n*100 >= box.Dx()*85 | |||
| 115 | + | } | |||
| 116 | + | thick, first := 0, 0 | |||
| 117 | + | for y := top; y <= bottom; y++ { | |||
| 118 | + | if y < bottom && rowOK(y) { | |||
| 119 | + | if thick == 0 { | |||
| 120 | + | first = y | |||
| 121 | + | } | |||
| 122 | + | thick++ | |||
| 123 | + | continue | |||
| 124 | + | } | |||
| 125 | + | if thick > 0 { | |||
| 126 | + | if thick > maxThick { | |||
| 127 | + | return underlineCandidate{}, false // a block, not a line | |||
| 128 | + | } | |||
| 129 | + | row := first + thick/2 | |||
| 130 | + | from, to := box.Min.X, box.Max.X | |||
| 131 | + | line := g.Pix[(row-g.Rect.Min.Y)*g.Stride:] | |||
| 132 | + | on := func(x int) bool { return abs(int(line[x-g.Rect.Min.X])-bg) >= thr } | |||
| 133 | + | for from > b.Min.X && on(from-1) { | |||
| 134 | + | from-- | |||
| 135 | + | } | |||
| 136 | + | for to < b.Max.X && on(to) { | |||
| 137 | + | to++ | |||
| 138 | + | } | |||
| 139 | + | return underlineCandidate{row: row, from: from, to: to}, true | |||
| 140 | + | } | |||
| 141 | + | } | |||
| 142 | + | return underlineCandidate{}, false | |||
| 143 | + | } | |||
| 144 | + | ||||
| 145 | + | // contrastOf returns the background level of box (the most common level, | |||
| 146 | + | // found in bins of 16 so antialiasing does not split it, then refined to the | |||
| 147 | + | // mean of that bin) and the contrast of its ink (the deviation reached by | |||
| 148 | + | // the strongest 3% of pixels). | |||
| 149 | + | func contrastOf(g *image.Gray, box image.Rectangle) (bg, ink int) { | |||
| 150 | + | var hist [256]int | |||
| 151 | + | for y := box.Min.Y; y < box.Max.Y; y++ { | |||
| 152 | + | for _, v := range g.Pix[(y-g.Rect.Min.Y)*g.Stride+box.Min.X-g.Rect.Min.X:][:box.Dx()] { | |||
| 153 | + | hist[v]++ | |||
| 154 | + | } | |||
| 155 | + | } | |||
| 156 | + | total := box.Dx() * box.Dy() | |||
| 157 | + | var bins [16]int | |||
| 158 | + | for v, c := range hist { | |||
| 159 | + | bins[v>>4] += c | |||
| 160 | + | } | |||
| 161 | + | best := 0 | |||
| 162 | + | for i, c := range bins { | |||
| 163 | + | if c > bins[best] { | |||
| 164 | + | best = i | |||
| 165 | + | } | |||
| 166 | + | } | |||
| 167 | + | sum, n := 0, 0 | |||
| 168 | + | for v := best << 4; v < (best+1)<<4; v++ { | |||
| 169 | + | sum += v * hist[v] | |||
| 170 | + | n += hist[v] | |||
| 171 | + | } | |||
| 172 | + | bg = sum / max(n, 1) | |||
| 173 | + | var dev [256]int | |||
| 174 | + | for v, c := range hist { | |||
| 175 | + | dev[abs(v-bg)] += c | |||
| 176 | + | } | |||
| 177 | + | n = 0 | |||
| 178 | + | for d := 255; d >= 0; d-- { | |||
| 179 | + | n += dev[d] | |||
| 180 | + | if n*100 >= total*3 { | |||
| 181 | + | return bg, d | |||
| 182 | + | } | |||
| 183 | + | } | |||
| 184 | + | return bg, 0 | |||
| 185 | + | } | |||
| 186 | + | ||||
| 187 | + | // underlinedLinks returns the targets for underlined words, joining the | |||
| 188 | + | // words of one continuous underline. A group that overlaps a URL target (in | |||
| 189 | + | // pixels) is left to it. Coordinates are scaled to logical. | |||
| 190 | + | func underlinedLinks(words []OCRWord, urls []image.Rectangle, scale float64) []ports.HintTarget { | |||
| 191 | + | var us []OCRWord | |||
| 192 | + | for _, w := range words { | |||
| 193 | + | if w.Underlined { | |||
| 194 | + | us = append(us, w) | |||
| 195 | + | } | |||
| 196 | + | } | |||
| 197 | + | sort.Slice(us, func(i, j int) bool { return us[i].Rect.Min.X < us[j].Rect.Min.X }) | |||
| 198 | + | used := make([]bool, len(us)) | |||
| 199 | + | var out []ports.HintTarget | |||
| 200 | + | for i := range us { | |||
| 201 | + | if used[i] { | |||
| 202 | + | continue | |||
| 203 | + | } | |||
| 204 | + | used[i] = true | |||
| 205 | + | rect, last := us[i].Rect, us[i] | |||
| 206 | + | chars, width := len(last.Text), last.Rect.Dx() | |||
| 207 | + | for { | |||
| 208 | + | next := -1 | |||
| 209 | + | for j := range us { | |||
| 210 | + | c := us[j] | |||
| 211 | + | if used[j] || c.Rect.Min.X < rect.Max.X-2 || abs(c.Rect.Max.Y-last.Rect.Max.Y) > max(3, rect.Dy()/3) { | |||
| 212 | + | continue | |||
| 213 | + | } | |||
| 214 | + | avg := float64(width+c.Rect.Dx()) / float64(max(1, chars+len(c.Text))) | |||
| 215 | + | gap := c.Rect.Min.X - rect.Max.X | |||
| 216 | + | if float64(gap) > 1.2*avg || c.Rect.Min.X > last.UnderR+2 { | |||
| 217 | + | continue | |||
| 218 | + | } | |||
| 219 | + | if next < 0 || c.Rect.Min.X < us[next].Rect.Min.X { | |||
| 220 | + | next = j | |||
| 221 | + | } | |||
| 222 | + | } | |||
| 223 | + | if next < 0 { | |||
| 224 | + | break | |||
| 225 | + | } | |||
| 226 | + | used[next] = true | |||
| 227 | + | last = us[next] | |||
| 228 | + | rect = rect.Union(last.Rect) | |||
| 229 | + | chars += len(last.Text) | |||
| 230 | + | width += last.Rect.Dx() | |||
| 231 | + | } | |||
| 232 | + | dup := false | |||
| 233 | + | for _, u := range urls { | |||
| 234 | + | if overlap(rect, u) > .5 { | |||
| 235 | + | dup = true | |||
| 236 | + | } | |||
| 237 | + | } | |||
| 238 | + | if dup { | |||
| 239 | + | continue | |||
| 240 | + | } | |||
| 241 | + | x, y := int(math.Floor(float64(rect.Min.X)/scale)), int(math.Floor(float64(rect.Min.Y)/scale)) | |||
| 242 | + | right, bottom := int(math.Ceil(float64(rect.Max.X)/scale)), int(math.Ceil(float64(rect.Max.Y)/scale)) | |||
| 243 | + | out = append(out, ports.HintTarget{Kind: ports.HintLink, Rect: ports.Rect{X: x, Y: y, W: right - x, H: bottom - y}}) | |||
| 244 | + | } | |||
| 245 | + | return out | |||
| 246 | + | } | |||
A internal/adapters/hints/underline_test.go +185 -0
| @@ -0,0 +1,185 @@ | |||
| 1 | + | package hints | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "image" | |
| 5 | + | "image/color" | |
| 6 | + | "os" | |
| 7 | + | "os/exec" | |
| 8 | + | "testing" | |
| 9 | + | ||
| 10 | + | "github.com/stretchr/testify/require" | |
| 11 | + | ||
| 12 | + | "github.com/bnema/neferwl/internal/ports" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | // canvas is a synthetic grayscale band: bg everywhere, ink for text. | |
| 16 | + | type canvas struct { | |
| 17 | + | g *image.Gray | |
| 18 | + | bg, ink uint8 | |
| 19 | + | s int // scale | |
| 20 | + | } | |
| 21 | + | ||
| 22 | + | func newCanvas(w, h int, bg, ink uint8, s int) *canvas { | |
| 23 | + | g := image.NewGray(image.Rect(0, 0, w*s, h*s)) | |
| 24 | + | for i := range g.Pix { | |
| 25 | + | g.Pix[i] = bg | |
| 26 | + | } | |
| 27 | + | return &canvas{g, bg, ink, s} | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | func (c *canvas) fill(x0, y0, x1, y1 int, v uint8) { | |
| 31 | + | for y := y0 * c.s; y < y1*c.s; y++ { | |
| 32 | + | for x := x0 * c.s; x < x1*c.s; x++ { | |
| 33 | + | c.g.SetGray(x, y, color.Gray{v}) | |
| 34 | + | } | |
| 35 | + | } | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | // word draws a 10 px high word of x-height body at (x,y) with the given | |
| 39 | + | // width: text-like noisy ink (a stem every 3 px) and optional descenders. | |
| 40 | + | // It returns the OCR box (descenders excluded, as a box around the body). | |
| 41 | + | func (c *canvas) word(x, y, w int, descender bool) OCRWord { | |
| 42 | + | for i := 0; i < w; i += 3 { | |
| 43 | + | c.fill(x+i, y, x+i+1, y+10, c.ink) | |
| 44 | + | } | |
| 45 | + | box := image.Rect(x*c.s, y*c.s, (x+w)*c.s, (y+10)*c.s) | |
| 46 | + | if descender { | |
| 47 | + | for i := 0; i < w; i += 6 { | |
| 48 | + | c.fill(x+i, y+10, x+i+1, y+13, c.ink) | |
| 49 | + | } | |
| 50 | + | box.Max.Y = (y + 13) * c.s | |
| 51 | + | } | |
| 52 | + | return OCRWord{Text: "word", Rect: box} | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | func (c *canvas) underline(x0, x1, y int) { c.fill(x0, y, x1, y+1, c.ink) } | |
| 56 | + | ||
| 57 | + | func scanUnder(c *canvas, words ...OCRWord) []ports.HintTarget { | |
| 58 | + | markUnderlined(c.g, words) | |
| 59 | + | return textLinks(words, float64(c.s)) | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | func TestUnderline(t *testing.T) { | |
| 63 | + | for _, tc := range []struct { | |
| 64 | + | name string | |
| 65 | + | bg, ink uint8 | |
| 66 | + | }{{"dark-bg", 10, 204}, {"light-bg", 245, 30}} { | |
| 67 | + | for _, s := range []int{1, 2} { | |
| 68 | + | t.Run(tc.name, func(t *testing.T) { | |
| 69 | + | mk := func() *canvas { return newCanvas(200, 60, tc.bg, tc.ink, s) } | |
| 70 | + | ||
| 71 | + | c := mk() | |
| 72 | + | w := c.word(10, 20, 40, false) | |
| 73 | + | c.underline(10, 50, 32) | |
| 74 | + | got := scanUnder(c, w) | |
| 75 | + | require.Len(t, got, 1) | |
| 76 | + | require.Equal(t, ports.HintLink, got[0].Kind) | |
| 77 | + | require.False(t, got[0].Text) | |
| 78 | + | require.Equal(t, ports.WindowID(0), got[0].Window) | |
| 79 | + | require.Equal(t, 10, got[0].Rect.X) | |
| 80 | + | ||
| 81 | + | c = mk() | |
| 82 | + | require.Empty(t, scanUnder(c, c.word(10, 20, 40, false)), "plain word") | |
| 83 | + | ||
| 84 | + | c = mk() | |
| 85 | + | require.Empty(t, scanUnder(c, c.word(10, 20, 40, true)), "descenders") | |
| 86 | + | ||
| 87 | + | c = mk() | |
| 88 | + | w = c.word(10, 20, 40, false) | |
| 89 | + | c.underline(0, 200, 32) | |
| 90 | + | require.Empty(t, scanUnder(c, w), "rule across") | |
| 91 | + | ||
| 92 | + | c = mk() | |
| 93 | + | w = c.word(10, 20, 40, false) | |
| 94 | + | c.fill(10, 24, 50, 25, c.ink) | |
| 95 | + | require.Empty(t, scanUnder(c, w), "strikethrough") | |
| 96 | + | ||
| 97 | + | // Two words with a continuous underline: one target. | |
| 98 | + | c = mk() | |
| 99 | + | a, b := c.word(10, 20, 40, false), c.word(56, 20, 40, false) | |
| 100 | + | c.underline(10, 96, 32) | |
| 101 | + | got = scanUnder(c, a, b) | |
| 102 | + | require.Len(t, got, 1) | |
| 103 | + | require.Equal(t, 10, got[0].Rect.X) | |
| 104 | + | require.Equal(t, 86, got[0].Rect.W) | |
| 105 | + | ||
| 106 | + | // A gap in the underline: two targets. | |
| 107 | + | c = mk() | |
| 108 | + | a, b = c.word(10, 20, 40, false), c.word(56, 20, 40, false) | |
| 109 | + | c.underline(10, 50, 32) | |
| 110 | + | c.underline(56, 96, 32) | |
| 111 | + | require.Len(t, scanUnder(c, a, b), 2) | |
| 112 | + | }) | |
| 113 | + | } | |
| 114 | + | } | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | // A box border under a word, with the word a part of a table row, is not a | |
| 118 | + | // link; neither is an underline that continues beyond into unrelated text. | |
| 119 | + | func TestUnderlineRejectsBox(t *testing.T) { | |
| 120 | + | c := newCanvas(300, 60, 10, 204, 1) | |
| 121 | + | w := c.word(100, 20, 40, false) | |
| 122 | + | c.underline(20, 280, 32) | |
| 123 | + | c.fill(20, 10, 21, 33, 204) | |
| 124 | + | c.fill(279, 10, 280, 33, 204) | |
| 125 | + | require.Empty(t, scanUnder(c, w)) | |
| 126 | + | } | |
| 127 | + | ||
| 128 | + | // The line must be thin. | |
| 129 | + | func TestUnderlineRejectsBlock(t *testing.T) { | |
| 130 | + | c := newCanvas(100, 60, 10, 204, 1) | |
| 131 | + | w := c.word(10, 20, 40, false) | |
| 132 | + | c.fill(10, 31, 50, 36, 204) | |
| 133 | + | require.Empty(t, scanUnder(c, w)) | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | func TestUnderlinedURLIsOneTextTarget(t *testing.T) { | |
| 137 | + | c := newCanvas(300, 60, 10, 204, 1) | |
| 138 | + | w := c.word(10, 20, 100, false) | |
| 139 | + | w.Text = "example.org" | |
| 140 | + | c.underline(10, 110, 32) | |
| 141 | + | got := scanUnder(c, w) | |
| 142 | + | require.Len(t, got, 1) | |
| 143 | + | require.True(t, got[0].Text) | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | func TestUnderlineOnBrowserTestdata(t *testing.T) { | |
| 147 | + | if _, err := exec.LookPath("tesseract"); err != nil { | |
| 148 | + | t.Skip("tesseract not on PATH") | |
| 149 | + | } | |
| 150 | + | f, err := os.Open("testdata/browser.png") | |
| 151 | + | require.NoError(t, err) | |
| 152 | + | defer f.Close() | |
| 153 | + | img, _, err := image.Decode(f) | |
| 154 | + | require.NoError(t, err) | |
| 155 | + | g := image.NewGray(img.Bounds()) | |
| 156 | + | for y := 0; y < g.Rect.Dy(); y++ { | |
| 157 | + | for x := 0; x < g.Rect.Dx(); x++ { | |
| 158 | + | g.Set(x, y, img.At(x, y)) | |
| 159 | + | } | |
| 160 | + | } | |
| 161 | + | words, _, err := readWords(t.Context(), g) | |
| 162 | + | require.NoError(t, err) | |
| 163 | + | var under, urls int | |
| 164 | + | for _, w := range words { | |
| 165 | + | if w.Underlined { | |
| 166 | + | under++ | |
| 167 | + | t.Logf("underlined: %q %v", w.Text, w.Rect) | |
| 168 | + | } | |
| 169 | + | } | |
| 170 | + | for _, tg := range textLinks(words, 1) { | |
| 171 | + | if tg.Text { | |
| 172 | + | urls++ | |
| 173 | + | } | |
| 174 | + | } | |
| 175 | + | t.Logf("words=%d underlined=%d url targets=%d total targets=%d", len(words), under, urls, len(textLinks(words, 1))) | |
| 176 | + | } | |
| 177 | + | ||
| 178 | + | // A single glyph with a flat bottom stroke (8, L) is not underlined text. | |
| 179 | + | func TestUnderlineIgnoresSingleGlyph(t *testing.T) { | |
| 180 | + | c := newCanvas(100, 60, 10, 204, 1) | |
| 181 | + | w := c.word(10, 20, 10, false) | |
| 182 | + | w.Text = "8" | |
| 183 | + | c.underline(10, 20, 32) | |
| 184 | + | require.Empty(t, scanUnder(c, w)) | |
| 185 | + | } | |
| @@ -0,0 +1,185 @@ | |||||
| 1 | + | package hints | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "image" | |||
| 5 | + | "image/color" | |||
| 6 | + | "os" | |||
| 7 | + | "os/exec" | |||
| 8 | + | "testing" | |||
| 9 | + | ||||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 11 | + | ||||
| 12 | + | "github.com/bnema/neferwl/internal/ports" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | // canvas is a synthetic grayscale band: bg everywhere, ink for text. | |||
| 16 | + | type canvas struct { | |||
| 17 | + | g *image.Gray | |||
| 18 | + | bg, ink uint8 | |||
| 19 | + | s int // scale | |||
| 20 | + | } | |||
| 21 | + | ||||
| 22 | + | func newCanvas(w, h int, bg, ink uint8, s int) *canvas { | |||
| 23 | + | g := image.NewGray(image.Rect(0, 0, w*s, h*s)) | |||
| 24 | + | for i := range g.Pix { | |||
| 25 | + | g.Pix[i] = bg | |||
| 26 | + | } | |||
| 27 | + | return &canvas{g, bg, ink, s} | |||
| 28 | + | } | |||
| 29 | + | ||||
| 30 | + | func (c *canvas) fill(x0, y0, x1, y1 int, v uint8) { | |||
| 31 | + | for y := y0 * c.s; y < y1*c.s; y++ { | |||
| 32 | + | for x := x0 * c.s; x < x1*c.s; x++ { | |||
| 33 | + | c.g.SetGray(x, y, color.Gray{v}) | |||
| 34 | + | } | |||
| 35 | + | } | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | // word draws a 10 px high word of x-height body at (x,y) with the given | |||
| 39 | + | // width: text-like noisy ink (a stem every 3 px) and optional descenders. | |||
| 40 | + | // It returns the OCR box (descenders excluded, as a box around the body). | |||
| 41 | + | func (c *canvas) word(x, y, w int, descender bool) OCRWord { | |||
| 42 | + | for i := 0; i < w; i += 3 { | |||
| 43 | + | c.fill(x+i, y, x+i+1, y+10, c.ink) | |||
| 44 | + | } | |||
| 45 | + | box := image.Rect(x*c.s, y*c.s, (x+w)*c.s, (y+10)*c.s) | |||
| 46 | + | if descender { | |||
| 47 | + | for i := 0; i < w; i += 6 { | |||
| 48 | + | c.fill(x+i, y+10, x+i+1, y+13, c.ink) | |||
| 49 | + | } | |||
| 50 | + | box.Max.Y = (y + 13) * c.s | |||
| 51 | + | } | |||
| 52 | + | return OCRWord{Text: "word", Rect: box} | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | func (c *canvas) underline(x0, x1, y int) { c.fill(x0, y, x1, y+1, c.ink) } | |||
| 56 | + | ||||
| 57 | + | func scanUnder(c *canvas, words ...OCRWord) []ports.HintTarget { | |||
| 58 | + | markUnderlined(c.g, words) | |||
| 59 | + | return textLinks(words, float64(c.s)) | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | func TestUnderline(t *testing.T) { | |||
| 63 | + | for _, tc := range []struct { | |||
| 64 | + | name string | |||
| 65 | + | bg, ink uint8 | |||
| 66 | + | }{{"dark-bg", 10, 204}, {"light-bg", 245, 30}} { | |||
| 67 | + | for _, s := range []int{1, 2} { | |||
| 68 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 69 | + | mk := func() *canvas { return newCanvas(200, 60, tc.bg, tc.ink, s) } | |||
| 70 | + | ||||
| 71 | + | c := mk() | |||
| 72 | + | w := c.word(10, 20, 40, false) | |||
| 73 | + | c.underline(10, 50, 32) | |||
| 74 | + | got := scanUnder(c, w) | |||
| 75 | + | require.Len(t, got, 1) | |||
| 76 | + | require.Equal(t, ports.HintLink, got[0].Kind) | |||
| 77 | + | require.False(t, got[0].Text) | |||
| 78 | + | require.Equal(t, ports.WindowID(0), got[0].Window) | |||
| 79 | + | require.Equal(t, 10, got[0].Rect.X) | |||
| 80 | + | ||||
| 81 | + | c = mk() | |||
| 82 | + | require.Empty(t, scanUnder(c, c.word(10, 20, 40, false)), "plain word") | |||
| 83 | + | ||||
| 84 | + | c = mk() | |||
| 85 | + | require.Empty(t, scanUnder(c, c.word(10, 20, 40, true)), "descenders") | |||
| 86 | + | ||||
| 87 | + | c = mk() | |||
| 88 | + | w = c.word(10, 20, 40, false) | |||
| 89 | + | c.underline(0, 200, 32) | |||
| 90 | + | require.Empty(t, scanUnder(c, w), "rule across") | |||
| 91 | + | ||||
| 92 | + | c = mk() | |||
| 93 | + | w = c.word(10, 20, 40, false) | |||
| 94 | + | c.fill(10, 24, 50, 25, c.ink) | |||
| 95 | + | require.Empty(t, scanUnder(c, w), "strikethrough") | |||
| 96 | + | ||||
| 97 | + | // Two words with a continuous underline: one target. | |||
| 98 | + | c = mk() | |||
| 99 | + | a, b := c.word(10, 20, 40, false), c.word(56, 20, 40, false) | |||
| 100 | + | c.underline(10, 96, 32) | |||
| 101 | + | got = scanUnder(c, a, b) | |||
| 102 | + | require.Len(t, got, 1) | |||
| 103 | + | require.Equal(t, 10, got[0].Rect.X) | |||
| 104 | + | require.Equal(t, 86, got[0].Rect.W) | |||
| 105 | + | ||||
| 106 | + | // A gap in the underline: two targets. | |||
| 107 | + | c = mk() | |||
| 108 | + | a, b = c.word(10, 20, 40, false), c.word(56, 20, 40, false) | |||
| 109 | + | c.underline(10, 50, 32) | |||
| 110 | + | c.underline(56, 96, 32) | |||
| 111 | + | require.Len(t, scanUnder(c, a, b), 2) | |||
| 112 | + | }) | |||
| 113 | + | } | |||
| 114 | + | } | |||
| 115 | + | } | |||
| 116 | + | ||||
| 117 | + | // A box border under a word, with the word a part of a table row, is not a | |||
| 118 | + | // link; neither is an underline that continues beyond into unrelated text. | |||
| 119 | + | func TestUnderlineRejectsBox(t *testing.T) { | |||
| 120 | + | c := newCanvas(300, 60, 10, 204, 1) | |||
| 121 | + | w := c.word(100, 20, 40, false) | |||
| 122 | + | c.underline(20, 280, 32) | |||
| 123 | + | c.fill(20, 10, 21, 33, 204) | |||
| 124 | + | c.fill(279, 10, 280, 33, 204) | |||
| 125 | + | require.Empty(t, scanUnder(c, w)) | |||
| 126 | + | } | |||
| 127 | + | ||||
| 128 | + | // The line must be thin. | |||
| 129 | + | func TestUnderlineRejectsBlock(t *testing.T) { | |||
| 130 | + | c := newCanvas(100, 60, 10, 204, 1) | |||
| 131 | + | w := c.word(10, 20, 40, false) | |||
| 132 | + | c.fill(10, 31, 50, 36, 204) | |||
| 133 | + | require.Empty(t, scanUnder(c, w)) | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | func TestUnderlinedURLIsOneTextTarget(t *testing.T) { | |||
| 137 | + | c := newCanvas(300, 60, 10, 204, 1) | |||
| 138 | + | w := c.word(10, 20, 100, false) | |||
| 139 | + | w.Text = "example.org" | |||
| 140 | + | c.underline(10, 110, 32) | |||
| 141 | + | got := scanUnder(c, w) | |||
| 142 | + | require.Len(t, got, 1) | |||
| 143 | + | require.True(t, got[0].Text) | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | func TestUnderlineOnBrowserTestdata(t *testing.T) { | |||
| 147 | + | if _, err := exec.LookPath("tesseract"); err != nil { | |||
| 148 | + | t.Skip("tesseract not on PATH") | |||
| 149 | + | } | |||
| 150 | + | f, err := os.Open("testdata/browser.png") | |||
| 151 | + | require.NoError(t, err) | |||
| 152 | + | defer f.Close() | |||
| 153 | + | img, _, err := image.Decode(f) | |||
| 154 | + | require.NoError(t, err) | |||
| 155 | + | g := image.NewGray(img.Bounds()) | |||
| 156 | + | for y := 0; y < g.Rect.Dy(); y++ { | |||
| 157 | + | for x := 0; x < g.Rect.Dx(); x++ { | |||
| 158 | + | g.Set(x, y, img.At(x, y)) | |||
| 159 | + | } | |||
| 160 | + | } | |||
| 161 | + | words, _, err := readWords(t.Context(), g) | |||
| 162 | + | require.NoError(t, err) | |||
| 163 | + | var under, urls int | |||
| 164 | + | for _, w := range words { | |||
| 165 | + | if w.Underlined { | |||
| 166 | + | under++ | |||
| 167 | + | t.Logf("underlined: %q %v", w.Text, w.Rect) | |||
| 168 | + | } | |||
| 169 | + | } | |||
| 170 | + | for _, tg := range textLinks(words, 1) { | |||
| 171 | + | if tg.Text { | |||
| 172 | + | urls++ | |||
| 173 | + | } | |||
| 174 | + | } | |||
| 175 | + | t.Logf("words=%d underlined=%d url targets=%d total targets=%d", len(words), under, urls, len(textLinks(words, 1))) | |||
| 176 | + | } | |||
| 177 | + | ||||
| 178 | + | // A single glyph with a flat bottom stroke (8, L) is not underlined text. | |||
| 179 | + | func TestUnderlineIgnoresSingleGlyph(t *testing.T) { | |||
| 180 | + | c := newCanvas(100, 60, 10, 204, 1) | |||
| 181 | + | w := c.word(10, 20, 10, false) | |||
| 182 | + | w.Text = "8" | |||
| 183 | + | c.underline(10, 20, 32) | |||
| 184 | + | require.Empty(t, scanUnder(c, w)) | |||
| 185 | + | } | |||
M internal/app/hintdebug.go +4 -8
| @@ -12,16 +12,12 @@ import ( | |||
| 12 | 12 | ||
| 13 | 13 | // hintOptions builds the hint worker options: the accessibility source, and, | |
| 14 | 14 | // when NEFERWL_HINT_DEBUG is "1", the dump directory of scanned frames, | |
| 15 | - | // $XDG_STATE_HOME/neferwl/hints. Without the accessibility bus the worker | |
| 16 | - | // reads uncovered windows with OCR only. | |
| 15 | + | // $XDG_STATE_HOME/neferwl/hints. The accessibility bus is only contacted | |
| 16 | + | // by the first scan; without it the worker reads windows with OCR only. | |
| 17 | 17 | func hintOptions(ctx context.Context) hints.Options { | |
| 18 | 18 | opts := hintDump(ctx) | |
| 19 | - | src, err := atspi.New(ctx) | |
| 20 | - | if err != nil { | |
| 21 | - | log := logging.For(ctx, "atspi") | |
| 22 | - | log.Warn().Err(err).Msg("accessibility hint targets unavailable, using OCR only") | |
| 23 | - | return opts | |
| 24 | - | } | |
| 19 | + | // The source connects on its first scan, and again after a failure. | |
| 20 | + | src := atspi.New() | |
| 25 | 21 | context.AfterFunc(ctx, func() { _ = src.Close() }) | |
| 26 | 22 | opts.Source = src | |
| 27 | 23 | return opts | |
| @@ -12,16 +12,12 @@ import ( | |||||
| 12 | 12 | ||||
| 13 | // hintOptions builds the hint worker options: the accessibility source, and, | 13 | // hintOptions builds the hint worker options: the accessibility source, and, | ||
| 14 | // when NEFERWL_HINT_DEBUG is "1", the dump directory of scanned frames, | 14 | // when NEFERWL_HINT_DEBUG is "1", the dump directory of scanned frames, | ||
| 15 | - | // $XDG_STATE_HOME/neferwl/hints. Without the accessibility bus the worker | 15 | + | // $XDG_STATE_HOME/neferwl/hints. The accessibility bus is only contacted |
| 16 | - | // reads uncovered windows with OCR only. | 16 | + | // by the first scan; without it the worker reads windows with OCR only. |
| 17 | func hintOptions(ctx context.Context) hints.Options { | 17 | func hintOptions(ctx context.Context) hints.Options { | ||
| 18 | opts := hintDump(ctx) | 18 | opts := hintDump(ctx) | ||
| 19 | - | src, err := atspi.New(ctx) | 19 | + | // The source connects on its first scan, and again after a failure. |
| 20 | - | if err != nil { | 20 | + | src := atspi.New() |
| 21 | - | log := logging.For(ctx, "atspi") | |||
| 22 | - | log.Warn().Err(err).Msg("accessibility hint targets unavailable, using OCR only") | |||
| 23 | - | return opts | |||
| 24 | - | } | |||
| 25 | context.AfterFunc(ctx, func() { _ = src.Close() }) | 21 | context.AfterFunc(ctx, func() { _ = src.Close() }) | ||
| 26 | opts.Source = src | 22 | opts.Source = src | ||
| 27 | return opts | 23 | return opts | ||
M internal/app/hintoptin_test.go +4 -0
| @@ -21,6 +21,8 @@ func TestHintPipesOffByDefault(t *testing.T) { | |||
| 21 | 21 | require.Nil(t, off.hintScans) | |
| 22 | 22 | require.Nil(t, off.hintResults) | |
| 23 | 23 | require.Nil(t, off.hintCaptured) | |
| 24 | + | // No demultiplexing: the outputs reply on the wayland channel itself. | |
| 25 | + | require.Equal(t, off.captured, off.capturedAll) | |
| 24 | 26 | ch := off.core() | |
| 25 | 27 | require.Nil(t, ch.HintScans) | |
| 26 | 28 | require.Nil(t, ch.Hints) | |
| @@ -28,6 +30,7 @@ func TestHintPipesOffByDefault(t *testing.T) { | |||
| 28 | 30 | require.NotNil(t, on.hintScans) | |
| 29 | 31 | require.NotNil(t, on.hintResults) | |
| 30 | 32 | require.NotNil(t, on.hintCaptured) | |
| 33 | + | require.NotEqual(t, on.captured, on.capturedAll) | |
| 31 | 34 | } | |
| 32 | 35 | ||
| 33 | 36 | func TestHintOffStartsNoHintGoroutine(t *testing.T) { | |
| @@ -43,6 +46,7 @@ func TestHintOffStartsNoHintGoroutine(t *testing.T) { | |||
| 43 | 46 | dump := string(buf[:runtime.Stack(buf, true)]) | |
| 44 | 47 | require.NotContains(t, dump, "adapters/hints.") | |
| 45 | 48 | require.NotContains(t, dump, "adapters/atspi.") | |
| 49 | + | require.NotContains(t, dump, "routeCaptured") | |
| 46 | 50 | select { | |
| 47 | 51 | case <-done: | |
| 48 | 52 | case <-time.After(3 * time.Second): | |
| @@ -21,6 +21,8 @@ func TestHintPipesOffByDefault(t *testing.T) { | |||||
| 21 | require.Nil(t, off.hintScans) | 21 | require.Nil(t, off.hintScans) | ||
| 22 | require.Nil(t, off.hintResults) | 22 | require.Nil(t, off.hintResults) | ||
| 23 | require.Nil(t, off.hintCaptured) | 23 | require.Nil(t, off.hintCaptured) | ||
| 24 | + | // No demultiplexing: the outputs reply on the wayland channel itself. | |||
| 25 | + | require.Equal(t, off.captured, off.capturedAll) | |||
| 24 | ch := off.core() | 26 | ch := off.core() | ||
| 25 | require.Nil(t, ch.HintScans) | 27 | require.Nil(t, ch.HintScans) | ||
| 26 | require.Nil(t, ch.Hints) | 28 | require.Nil(t, ch.Hints) | ||
| @@ -28,6 +30,7 @@ func TestHintPipesOffByDefault(t *testing.T) { | |||||
| 28 | require.NotNil(t, on.hintScans) | 30 | require.NotNil(t, on.hintScans) | ||
| 29 | require.NotNil(t, on.hintResults) | 31 | require.NotNil(t, on.hintResults) | ||
| 30 | require.NotNil(t, on.hintCaptured) | 32 | require.NotNil(t, on.hintCaptured) | ||
| 33 | + | require.NotEqual(t, on.captured, on.capturedAll) | |||
| 31 | } | 34 | } | ||
| 32 | 35 | ||||
| 33 | func TestHintOffStartsNoHintGoroutine(t *testing.T) { | 36 | func TestHintOffStartsNoHintGoroutine(t *testing.T) { | ||
| @@ -43,6 +46,7 @@ func TestHintOffStartsNoHintGoroutine(t *testing.T) { | |||||
| 43 | dump := string(buf[:runtime.Stack(buf, true)]) | 46 | dump := string(buf[:runtime.Stack(buf, true)]) | ||
| 44 | require.NotContains(t, dump, "adapters/hints.") | 47 | require.NotContains(t, dump, "adapters/hints.") | ||
| 45 | require.NotContains(t, dump, "adapters/atspi.") | 48 | require.NotContains(t, dump, "adapters/atspi.") | ||
| 49 | + | require.NotContains(t, dump, "routeCaptured") | |||
| 46 | select { | 50 | select { | ||
| 47 | case <-done: | 51 | case <-done: | ||
| 48 | case <-time.After(3 * time.Second): | 52 | case <-time.After(3 * time.Second): | ||
M internal/app/session.go +2 -3
| @@ -346,10 +346,9 @@ func (s *session) run(ctx context.Context, inject func(chan<- ports.InputEvent)) | |||
| 346 | 346 | log.Warn().Err(err).Msg("capture allowlist not watched, restart to apply changes") | |
| 347 | 347 | } | |
| 348 | 348 | }() | |
| 349 | - | workers.Add(1) | |
| 350 | - | go func() { defer workers.Done(); routeCaptured(ctx, p.capturedAll, p.captured, p.internalCaptured()) }() | |
| 351 | 349 | if p.hintScans != nil { | |
| 352 | - | workers.Add(1) | |
| 350 | + | workers.Add(2) | |
| 351 | + | go func() { defer workers.Done(); routeCaptured(ctx, p.capturedAll, p.captured, p.internalCaptured()) }() | |
| 353 | 352 | go func() { | |
| 354 | 353 | defer workers.Done() | |
| 355 | 354 | // Run only returns when ctx is done: nothing to report. | |
| @@ -346,10 +346,9 @@ func (s *session) run(ctx context.Context, inject func(chan<- ports.InputEvent)) | |||||
| 346 | log.Warn().Err(err).Msg("capture allowlist not watched, restart to apply changes") | 346 | log.Warn().Err(err).Msg("capture allowlist not watched, restart to apply changes") | ||
| 347 | } | 347 | } | ||
| 348 | }() | 348 | }() | ||
| 349 | - | workers.Add(1) | |||
| 350 | - | go func() { defer workers.Done(); routeCaptured(ctx, p.capturedAll, p.captured, p.internalCaptured()) }() | |||
| 351 | if p.hintScans != nil { | 349 | if p.hintScans != nil { | ||
| 352 | - | workers.Add(1) | 350 | + | workers.Add(2) |
| 351 | + | go func() { defer workers.Done(); routeCaptured(ctx, p.capturedAll, p.captured, p.internalCaptured()) }() | |||
| 353 | go func() { | 352 | go func() { | ||
| 354 | defer workers.Done() | 353 | defer workers.Done() | ||
| 355 | // Run only returns when ctx is done: nothing to report. | 354 | // Run only returns when ctx is done: nothing to report. | ||
M internal/app/wiring.go +9 -5
| @@ -53,9 +53,10 @@ type pipes struct { | |||
| 53 | 53 | captures chan ports.CaptureRequest | |
| 54 | 54 | // Every admitted capture owns one reply slot until routeCaptured takes | |
| 55 | 55 | // it. Keep this capacity tied to admission so output routing never waits. | |
| 56 | - | // The outputs reply on capturedAll; routeCaptured splits the replies by | |
| 56 | + | // With hint mode on, the outputs reply on capturedAll and routeCaptured splits the replies by | |
| 57 | 57 | // ports.CaptureIDInternal into captured (the wayland consumer) and | |
| 58 | - | // hintCaptured (internal consumers). | |
| 58 | + | // hintCaptured (internal consumers). With hint mode off, capturedAll is | |
| 59 | + | // captured itself and there is no routeCaptured. | |
| 59 | 60 | capturedAll chan ports.CaptureDone | |
| 60 | 61 | captured chan ports.CaptureDone | |
| 61 | 62 | // hintCaptured carries internal capture replies to the hints adapter. | |
| @@ -110,9 +111,8 @@ func newPipes(realSession, hint bool) *pipes { | |||
| 110 | 111 | ||
| 111 | 112 | frames: make(chan ports.OutputFrame, 8), | |
| 112 | 113 | ||
| 113 | - | captures: make(chan ports.CaptureRequest, ports.MaxCaptureInflight), | |
| 114 | - | capturedAll: make(chan ports.CaptureDone, ports.MaxCaptureInflight), | |
| 115 | - | captured: make(chan ports.CaptureDone, ports.MaxCaptureInflight), | |
| 114 | + | captures: make(chan ports.CaptureRequest, ports.MaxCaptureInflight), | |
| 115 | + | captured: make(chan ports.CaptureDone, ports.MaxCaptureInflight), | |
| 116 | 116 | ||
| 117 | 117 | outputFormats: make(chan ports.OutputFormats, 8), | |
| 118 | 118 | outputHeads: make(chan ports.OutputHeads, 8), | |
| @@ -126,7 +126,11 @@ func newPipes(realSession, hint bool) *pipes { | |||
| 126 | 126 | ||
| 127 | 127 | castCommands: make(chan ports.CastCommand, 64), | |
| 128 | 128 | } | |
| 129 | + | // Without hint mode every reply is for the wayland consumer: the outputs | |
| 130 | + | // reply on its channel directly and nothing demultiplexes. | |
| 131 | + | p.capturedAll = p.captured | |
| 129 | 132 | if hint { | |
| 133 | + | p.capturedAll = make(chan ports.CaptureDone, ports.MaxCaptureInflight) | |
| 130 | 134 | p.hintScans = make(chan ports.HintScanRequest, 1) | |
| 131 | 135 | p.hintResults = make(chan ports.HintScanResult, 1) | |
| 132 | 136 | p.hintCaptured = make(chan ports.CaptureDone, 1) | |
| @@ -53,9 +53,10 @@ type pipes struct { | |||||
| 53 | captures chan ports.CaptureRequest | 53 | captures chan ports.CaptureRequest | ||
| 54 | // Every admitted capture owns one reply slot until routeCaptured takes | 54 | // Every admitted capture owns one reply slot until routeCaptured takes | ||
| 55 | // it. Keep this capacity tied to admission so output routing never waits. | 55 | // it. Keep this capacity tied to admission so output routing never waits. | ||
| 56 | - | // The outputs reply on capturedAll; routeCaptured splits the replies by | 56 | + | // With hint mode on, the outputs reply on capturedAll and routeCaptured splits the replies by |
| 57 | // ports.CaptureIDInternal into captured (the wayland consumer) and | 57 | // ports.CaptureIDInternal into captured (the wayland consumer) and | ||
| 58 | - | // hintCaptured (internal consumers). | 58 | + | // hintCaptured (internal consumers). With hint mode off, capturedAll is |
| 59 | + | // captured itself and there is no routeCaptured. | |||
| 59 | capturedAll chan ports.CaptureDone | 60 | capturedAll chan ports.CaptureDone | ||
| 60 | captured chan ports.CaptureDone | 61 | captured chan ports.CaptureDone | ||
| 61 | // hintCaptured carries internal capture replies to the hints adapter. | 62 | // hintCaptured carries internal capture replies to the hints adapter. | ||
| @@ -110,9 +111,8 @@ func newPipes(realSession, hint bool) *pipes { | |||||
| 110 | 111 | ||||
| 111 | frames: make(chan ports.OutputFrame, 8), | 112 | frames: make(chan ports.OutputFrame, 8), | ||
| 112 | 113 | ||||
| 113 | - | captures: make(chan ports.CaptureRequest, ports.MaxCaptureInflight), | 114 | + | captures: make(chan ports.CaptureRequest, ports.MaxCaptureInflight), |
| 114 | - | capturedAll: make(chan ports.CaptureDone, ports.MaxCaptureInflight), | 115 | + | captured: make(chan ports.CaptureDone, ports.MaxCaptureInflight), |
| 115 | - | captured: make(chan ports.CaptureDone, ports.MaxCaptureInflight), | |||
| 116 | 116 | ||||
| 117 | outputFormats: make(chan ports.OutputFormats, 8), | 117 | outputFormats: make(chan ports.OutputFormats, 8), | ||
| 118 | outputHeads: make(chan ports.OutputHeads, 8), | 118 | outputHeads: make(chan ports.OutputHeads, 8), | ||
| @@ -126,7 +126,11 @@ func newPipes(realSession, hint bool) *pipes { | |||||
| 126 | 126 | ||||
| 127 | castCommands: make(chan ports.CastCommand, 64), | 127 | castCommands: make(chan ports.CastCommand, 64), | ||
| 128 | } | 128 | } | ||
| 129 | + | // Without hint mode every reply is for the wayland consumer: the outputs | |||
| 130 | + | // reply on its channel directly and nothing demultiplexes. | |||
| 131 | + | p.capturedAll = p.captured | |||
| 129 | if hint { | 132 | if hint { | ||
| 133 | + | p.capturedAll = make(chan ports.CaptureDone, ports.MaxCaptureInflight) | |||
| 130 | p.hintScans = make(chan ports.HintScanRequest, 1) | 134 | p.hintScans = make(chan ports.HintScanRequest, 1) | ||
| 131 | p.hintResults = make(chan ports.HintScanResult, 1) | 135 | p.hintResults = make(chan ports.HintScanResult, 1) | ||
| 132 | p.hintCaptured = make(chan ports.CaptureDone, 1) | 136 | p.hintCaptured = make(chan ports.CaptureDone, 1) | ||