Skip to content

Commit

Permalink
cool#11064 kit: check for callbacks & wsd socket in the anyInput call…
Browse files Browse the repository at this point in the history
…back

Open the 300 pages bugdoc, go to the end of page 1 (before a page
break), press enter, a page 2 is inserted and we get new tiles after a
delay:
15:08:33.803 global.js:566 1738073313803 OUTGOING: key type=up char=0 key=1280
...
15:08:34.249 global.js:566 1738073314249 INCOMING: delta: nviewid=1000 part=0 ...
-> 446 ms.

The reason this happens is because our anyInput callback tells core to
finish the idle Writer layout first and only then render the tiles for
the visible area, which is a problem similar to what commit
930cd61 (Related: cool#9735 kit: use
the new registerAnyInputCallback() LOK API, 2024-08-26) fixed.

Fix this problem by improving the anyInput callbacks to take 2 more
"inputs" into account:
1) LOK_CALLBACK_INVALIDATE_TILES went to KitQueue::_callbacks and this
   was only processed in the next main loop iteration, so checks for
  this in the anyInput callback, which partly reverts commit
  2823933 (anyInput - should only depend
  on incoming messages from coolwsd., 2024-12-10).
2) Now that the invalidation is processed, it's sent to the wsd process
   and we get a tile request on the wsd -> kit socket, in the kit
  process. Fix this problem by doing a poll() on that socket as the
  already existing FIXME comment suggested.

With this, we paint tiles for the visible area before calculating pages
3..300 in Writer:
10:15:47.441 global.js:566 1738314947441 OUTGOING: key type=input char=13 key=1280
...
10:15:47.476 global.js:566 1738314947476 INCOMING: delta: nviewid=1000 part=0 ...
-> 24ms

Signed-off-by: Miklos Vajna <[email protected]>
Change-Id: I668d2ebf341b628841e0a637fee9e476eeddd179
  • Loading branch information
vmiklos committed Jan 31, 2025
1 parent 2a318fc commit 331e495
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 5 deletions.
24 changes: 22 additions & 2 deletions kit/Kit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3001,13 +3001,33 @@ int pollCallback(void* data, int timeoutUs)
}

// Do we have any pending input events from coolwsd ?
// FIXME: we could helpfully poll our incoming socket too here.
bool anyInputCallback(void* data)
{
auto kitSocketPoll = reinterpret_cast<KitSocketPoll*>(data);
std::shared_ptr<Document> document = kitSocketPoll->getDocument();

return document && document->hasQueueItems();
if (document)
{
if (document->hasCallbacks())
{
// Have pending LOK callbacks from core.
return true;
}

// Poll our incoming socket from wsd.
int ret = kitSocketPoll->poll(std::chrono::microseconds(0), /*justPoll=*/true);
if (ret)
{
return true;
}

if (document->hasQueueItems())
{
return true;
}
}

return false;
}

/// Called by LOK main-loop
Expand Down
8 changes: 7 additions & 1 deletion net/Socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ void SocketPoll::enableWatchdog()
_watchdogTime = Watchdog::getTimestamp();
}

int SocketPoll::poll(int64_t timeoutMaxMicroS)
int SocketPoll::poll(int64_t timeoutMaxMicroS, bool justPoll)
{
if (_runOnClientThread)
checkAndReThread();
Expand Down Expand Up @@ -526,6 +526,12 @@ int SocketPoll::poll(int64_t timeoutMaxMicroS)
// from now we want to race back to sleep.
enableWatchdog();

if (justPoll && size > 0)
{
// Done with the poll(), don't process anything.
return _pollFds[0].revents;
}

// First process the wakeup pipe (always the last entry).
if (_pollFds[size].revents)
{
Expand Down
4 changes: 2 additions & 2 deletions net/Socket.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,7 @@ class SocketPoll
/// Poll the sockets for available data to read or buffer to write.
/// Returns the return-value of poll(2): 0 on timeout,
/// -1 for error, and otherwise the number of events signalled.
int poll(std::chrono::microseconds timeoutMax) { return poll(timeoutMax.count()); }
int poll(std::chrono::microseconds timeoutMax, bool justPoll = false) { return poll(timeoutMax.count(), justPoll); }

/// Poll the sockets for available data to read or buffer to write.
/// Returns the return-value of poll(2): 0 on timeout,
Expand Down Expand Up @@ -956,7 +956,7 @@ class SocketPoll
}

/// Actual poll implementation
int poll(int64_t timeoutMaxMicroS);
int poll(int64_t timeoutMaxMicroS, bool justPoll = false);

/// Initialize the poll fds array with the right events
void setupPollFds(std::chrono::steady_clock::time_point now,
Expand Down

0 comments on commit 331e495

Please sign in to comment.