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

Rendering should be virtual wrt viewport #5

Closed
joaomoreno opened this issue Jun 2, 2020 · 5 comments
Closed

Rendering should be virtual wrt viewport #5

joaomoreno opened this issue Jun 2, 2020 · 5 comments
Assignees

Comments

@joaomoreno
Copy link
Member

joaomoreno commented Jun 2, 2020

Testing microsoft/vscode#98980

You should be able to use our list widget for this. It seems we're rendering wayyy more than necessary and I get a lot of blank frames when scrolling around.

@lramos15
Copy link
Member

lramos15 commented Jun 2, 2020

This was discussed briefly and I think it was just stated that it was too intertwined with the core of VS to easily utilize in the hex editor. I would be happy to discuss it more with you if you have any further knowledge about how that could easily be done, I would be happy to discuss it as my implementation does do some basic chunk rendering which does have a decent buffer outside the viewport.

@joaomoreno
Copy link
Member Author

joaomoreno commented Jun 2, 2020

We'd have to weed it out but it is feasible! We even have paging capabilities which simply calls for pages to load, as the list scrolls up and down.

@rebornix
Copy link
Member

rebornix commented Jun 2, 2020

recording

I'm seeing a lot of blank frames when scrolling even though the file is relatively small (2k loc), which makes navigating in the document really hard.

If adopting list view is hard, I think we should at least add some gpu optimization to the rendering.

@rebornix
Copy link
Member

rebornix commented Jun 2, 2020

I think we should at least add some gpu optimization to the rendering.

I took it back, the root cause is forced reflow -->

image

private populateHexAdresses(rowData: VirtualizedPacket[]): void {
        const hex_addr = document.getElementById("hexaddr");
        const offset = rowData[0].offset;
        const addr = document.createElement("div");
        addr.className = "row";
        addr.setAttribute("data-offset", offset.toString());
        addr.innerText = pad(offset.toString(16), 8).toUpperCase();
        hex_addr!.appendChild(addr);
        this.rows[0].set(offset.toString(), addr);
        // We add a left px offset to effectively right align the column
        addr.style.left = `${addr.parentElement!.clientWidth - addr.clientWidth}px`;
        this.translateRow(addr, offset);
    }

Suggestions:

  • hold a reference to hexaddr DOM node
  • cache its width
  • listen to window size change and then update the cache (and also rerender the viewport if necessary)

I think this will remove most force reflow and reduce this issue by a ton.

@connor4312
Copy link
Member

We're now virtual

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants