Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tiles: duplicated rendering #11134

Merged
merged 6 commits into from
Feb 12, 2025
Merged

Conversation

eszkadev
Copy link
Contributor

@eszkadev eszkadev commented Feb 11, 2025

Related to #10980
Check if we have pending rendering before requesting new one for the same tile.
Add some useful logging for rare cases in TRC.
Client only "can" request rendering keyframe by wid=0 when tile is not in cache.
Client cannot force it when we already have correct one.

Manual test:

  1. open calc
  2. open debug tools with tile overlays and invalidations
  3. hold page down until reaching missing tiles (red color)
    Check what we received, ideally would be to just get single tile as it wasn't edited.

5-after-page-down

@eszkadev eszkadev changed the title tiles: duplicated rendering tiles: duplicated rendering #10980 Feb 11, 2025
@eszkadev eszkadev changed the title tiles: duplicated rendering #10980 tiles: duplicated rendering Feb 11, 2025
@eszkadev eszkadev requested review from mmeeks and caolanm and removed request for mmeeks February 11, 2025 15:06
Copy link
Contributor

@mmeeks mmeeks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine for me on red - would be good to tweak re: Caolan's comment.

if (!cachedTile || tooLarge)
{
forceKeyFrame = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sensible to me =) I was worried about the race between the kit and coolwsd but we request this tile anyway I guess.

@mmeeks
Copy link
Contributor

mmeeks commented Feb 12, 2025

Looking good to me for an early merge on red - thanks Szymon =) !

@eszkadev eszkadev force-pushed the private/eszkadev/tiles-duplicated branch from 39e70e8 to cd88042 Compare February 12, 2025 14:37
Just logging so we know if cache cleanup was done before.
That will tell us if we might want to actively render something.
If tile cache was empty, we can just now schedule a new render.
It can be useful:
- on initial load (nothing in cache yet)
- on cache cleanup (we lost old tiles)

Signed-off-by: Szymon Kłos <[email protected]>
Change-Id: I116f80dcba5d5fd70bb00dfa8823781cd0aa2596
Signed-off-by: Szymon Kłos <[email protected]>
Change-Id: I99c43ee4fb79d422b336b625b7b1a2599837cbce
related to #10980

open calc and use page down to enter some initially not visible area
notice in debug tools that tiles are rendered twice

If any tile rendering is already pending - let's don't request
it again. Use shared function for that check which was used only
in one place but forgotten in hadnling the incoming tilecombine message.

Signed-off-by: Szymon Kłos <[email protected]>
Change-Id: Icc1bf97826ec8b1cb1185b6f30dc2daf6d0355ea
Signed-off-by: Szymon Kłos <[email protected]>
Change-Id: I7cbb8524db50cfc38d68d8c949d6637293226ce8
Signed-off-by: Szymon Kłos <[email protected]>
Change-Id: Ie890924f7f8c307f18a79726ddc390bbf7c74965
Signed-off-by: Szymon Kłos <[email protected]>
Change-Id: Ie908ee8369339ee4633bbc9251ac03ba460516e8
@eszkadev eszkadev force-pushed the private/eszkadev/tiles-duplicated branch from bd0d5fe to cf26cc1 Compare February 12, 2025 15:40
@eszkadev eszkadev merged commit f5edabd into master Feb 12, 2025
14 checks passed
@eszkadev eszkadev deleted the private/eszkadev/tiles-duplicated branch February 12, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants