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

WebGL typescript refactor and add quad glyph #12046

Merged
merged 7 commits into from
Mar 17, 2022
Merged

Conversation

ianthomas23
Copy link
Member

Following on from the WebGL shader refactor for markers (#12040), this PR refactors the typescript for WebGL markers (and other fixed-shaped glyphs) and adds a WebGL quad glyph. Class hierarchy is now

For markers, including rect, circle, etc, there is now only a single place where the ReGL draw functions are called, and only a single place where the line/fill/hatch visuals are setup for WebGL. The concrete marker classes just set the locations (center, width, height and angles) of the WebGL glyphs.

I have added a new quad glyph to show this works in ~60 lines of typescript, and extended the existing quad visual test to include WebGL; you can tell it is not just a copy of the canvas rendering by the slightly different antialiased corner pixels.

I have also put WebGL circle in its own class rather than sharing what is now called the MultiMarkerGL class which makes the latter easier to understand.

One thing I have noticed is that the antialiasing width is different for scatter markers than for rect, etc and they should really be the same. This is historical and I should have fix it them #12040. I have kept them different for the moment as to make them the same will change many WebGL visual baseline images. There are 2 options for dealing with this:

  1. Wait until I next have to change many WebGL baseline images anyway, and include this fix at the same time.
  2. Change it now anyway and take the hit of the baseline image changes before 3.0 release.
    I am happy to accept advice on this.

Following on from this it will be easy to add Hbar, Vbar and HexTile WebGL glyphs.

This work is part of the CZI round 3 extension.

@mattpap mattpap added this to the 3.0 milestone Mar 15, 2022
@ianthomas23
Copy link
Member Author

On further consideration, let's keep the different antialiasing width as it is here. I need to write a PR fixing the different line joins for WebGL markers and that will change quite a few WebGL marker baseline images, so I can correct the antialiasing width at the same time. I'll do that in the next few weeks.

@mattpap
Copy link
Contributor

mattpap commented Mar 16, 2022

Wait until I next have to change many WebGL baseline images anyway, and include this fix at the same time.

I suppose it isn't a very visible difference? If so, we can safely wait for another batch of changes.

@ianthomas23
Copy link
Member Author

I suppose it isn't a very visible difference? If so, we can safely wait for another batch of changes.

Yes, it is just the antialiased edge pixels that have slightly different intensities.

@mattpap mattpap merged commit c2e2b16 into branch-3.0 Mar 17, 2022
@mattpap mattpap deleted the ianthomas23/webgl_quad branch March 17, 2022 10:41
ianthomas23 added a commit that referenced this pull request May 16, 2022
* SingleMarkerGL for rect and quad

* MarkerGL inherits from BaseMarkerGL

* Better class hierarchy

* Test WebGL quad glyph

* Separate CircleGL class

* Avoid use of any

* Remove code duplication
@ianthomas23 ianthomas23 modified the milestones: 3.0, 2.4.3 May 16, 2022
bryevdv pushed a commit that referenced this pull request May 16, 2022
* Fix webgl line NaN handling (#11829)

* Fix webgl line nan handling

* Add baseline images

* Support multiple marker types in WebGL scatter (#11840)

* Support multiple marker types in WebGL scatter

* Update macos/windows baseline images

* Changes following review

* Add docs about different render order

* Remove temporary webgl circle fix (#11849)

* Add regression test for updating webgl line (#11884)

* Add regression test for updating webgl line (issue #8346)

* Update macos and windows baseline images

* Unify WebGL markers and rects (#12040)

* Unify markers and rects

* Baseline images

* Consistent code style

* WebGL typescript refactor and add quad glyph (#12046)

* SingleMarkerGL for rect and quad

* MarkerGL inherits from BaseMarkerGL

* Better class hierarchy

* Test WebGL quad glyph

* Separate CircleGL class

* Avoid use of any

* Remove code duplication

* Implement WebGL marker line joins (#12055)

* Line joins for markers

* Use same antialiasing width for all webgl markers

* Correct size hints

* Baseline image changes

* Marker glyph should support line join test

* WebGL support for HBar VBar and HexTile glyphs (#12061)

* Add WebGL HBar and VBar glyphs

* Add WebGL HexTile glyphs

* Visual test changes

* Include rotation in should_support_Rect test

* Minor WebGL line refactor (#12120)

* Fix Wedge and AnnualWedge glyph hitboxes (#12071)

* Fix Wedge and AnnualWedge glyph hitboxes

* Hitbox tests for Wedge and AnnualWedge glyphs

Co-authored-by: Michael Brown <michael@msbrown.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants