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

line chart: introduce onContextLost renderer callback #5237

Merged
merged 1 commit into from
Aug 16, 2021

Conversation

psybuzz
Copy link
Contributor

@psybuzz psybuzz commented Aug 12, 2021

Each line chart component in the Time Series dashboard gets its own canvas and
webgl rendering context. After scrolling through too many line charts (for my
device after ~30), the page issues warnings in the console like "Too many active
WebGL contexts. Oldest context will be lost".

When the browser decides there are too many contexts, it disposes old ones
causing blank line charts. This change adds an event dispatched by the line chart's
internal ChartImpl to notify clients when the rendering context is lost.

This only applies to the WebGL renderer for now. A followup change addresses the
remediation of blank scalar cards.

Googlers, see b/196294346.

Diffbase: none.
Followup: #5239

@google-cla google-cla bot added the cla: yes label Aug 12, 2021
@psybuzz psybuzz marked this pull request as ready for review August 12, 2021 16:45
@psybuzz psybuzz marked this pull request as draft August 12, 2021 16:59
@psybuzz psybuzz changed the title timeseries: restore line charts that lose webgl context line chart: introduce onContextLost renderer callback Aug 13, 2021
@psybuzz psybuzz marked this pull request as ready for review August 13, 2021 00:53
Copy link
Contributor

@japie1235813 japie1235813 left a comment

Choose a reason for hiding this comment

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

The structure lgtm.
What thing is not clear to me: what do you think the app should do when the context lost happens?

@psybuzz
Copy link
Contributor Author

psybuzz commented Aug 16, 2021

what do you think the app should do when the context lost happens?

Upon losing webgl context, my intention is to have the LineChartComponent listen for that renderer event and tell its parent component that a "fatal renderer error" has occurred. In that scenario, a ScalarCardComponent, which owns a LineChartComponent, can decide to destroy and re-create the LineChartComponent upon a fatal error.

Sorry, I realize it wasn't simple to view the followup PR's changes independently. I will rebase the followup before requesting review, and remember to use branches in the main repo for easier diffing in the future.

@psybuzz psybuzz merged commit 7660f72 into tensorflow:master Aug 16, 2021
psybuzz added a commit that referenced this pull request Aug 18, 2021
The line chart's internal `ChartImpl` notifies the line chart when it is a WebGL
renderer that loses its context due to the page having too many active WebGL
contexts at once (>30 for my device).

This change adds logic to treat those errors as fatal when the line chart needs
to be updated.  Scalar cards now handle the fatal error by destroying and
recreating the entire line chart component, fixing the blank chart issue.

Manually checked that charts are not blank after scrolling through lots of line
charts (used the tagFilter to view 2x the line charts, since elements are
duplicated).

Alternatives considered: calling restoreContext() [1] on the context does not
consistently fix the blank charts.  When it does, it appears to lose the original
context configuration (antialias: true, precision: 'highp', alpha: true), leading to
low quality visuals [2].

Googlers, see b/196294346.

[1] https://developer.mozilla.org/en-US/docs/Web/API/WEBGL_lose_context/restoreContext
[2] https://imgur.com/a/kKnhqYJ

Diffbase: #5237
yatbear pushed a commit to yatbear/tensorboard that referenced this pull request Mar 27, 2023
Each line chart component in the Time Series dashboard gets its own canvas and
webgl rendering context. After scrolling through too many line charts (for my
device after ~30), the page issues warnings in the console like "Too many active
WebGL contexts. Oldest context will be lost".

When the browser decides there are too many contexts, it disposes old ones
causing blank line charts. This change adds an event dispatched by the line chart's
internal `ChartImpl` to notify clients when the rendering context is lost.

This only applies to the WebGL renderer for now.  A followup change addresses the
remediation of blank scalar cards.

Googlers, see b/196294346.
Followup: tensorflow#5239
yatbear pushed a commit to yatbear/tensorboard that referenced this pull request Mar 27, 2023
The line chart's internal `ChartImpl` notifies the line chart when it is a WebGL
renderer that loses its context due to the page having too many active WebGL
contexts at once (>30 for my device).

This change adds logic to treat those errors as fatal when the line chart needs
to be updated.  Scalar cards now handle the fatal error by destroying and
recreating the entire line chart component, fixing the blank chart issue.

Manually checked that charts are not blank after scrolling through lots of line
charts (used the tagFilter to view 2x the line charts, since elements are
duplicated).

Alternatives considered: calling restoreContext() [1] on the context does not
consistently fix the blank charts.  When it does, it appears to lose the original
context configuration (antialias: true, precision: 'highp', alpha: true), leading to
low quality visuals [2].

Googlers, see b/196294346.

[1] https://developer.mozilla.org/en-US/docs/Web/API/WEBGL_lose_context/restoreContext
[2] https://imgur.com/a/kKnhqYJ

Diffbase: tensorflow#5237
dna2github pushed a commit to dna2fork/tensorboard that referenced this pull request May 1, 2023
Each line chart component in the Time Series dashboard gets its own canvas and
webgl rendering context. After scrolling through too many line charts (for my
device after ~30), the page issues warnings in the console like "Too many active
WebGL contexts. Oldest context will be lost".

When the browser decides there are too many contexts, it disposes old ones
causing blank line charts. This change adds an event dispatched by the line chart's
internal `ChartImpl` to notify clients when the rendering context is lost.

This only applies to the WebGL renderer for now.  A followup change addresses the
remediation of blank scalar cards.

Googlers, see b/196294346.
Followup: tensorflow#5239
dna2github pushed a commit to dna2fork/tensorboard that referenced this pull request May 1, 2023
The line chart's internal `ChartImpl` notifies the line chart when it is a WebGL
renderer that loses its context due to the page having too many active WebGL
contexts at once (>30 for my device).

This change adds logic to treat those errors as fatal when the line chart needs
to be updated.  Scalar cards now handle the fatal error by destroying and
recreating the entire line chart component, fixing the blank chart issue.

Manually checked that charts are not blank after scrolling through lots of line
charts (used the tagFilter to view 2x the line charts, since elements are
duplicated).

Alternatives considered: calling restoreContext() [1] on the context does not
consistently fix the blank charts.  When it does, it appears to lose the original
context configuration (antialias: true, precision: 'highp', alpha: true), leading to
low quality visuals [2].

Googlers, see b/196294346.

[1] https://developer.mozilla.org/en-US/docs/Web/API/WEBGL_lose_context/restoreContext
[2] https://imgur.com/a/kKnhqYJ

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

Successfully merging this pull request may close these issues.

2 participants