GPU Aggregation (2/8): GPUAggregator #8887
Conversation
Pessimistress
force-pushed
the
x/aggregation-2
branch
from
2362a0d
to
430a347
Compare
Pessimistress
force-pushed
the
x/aggregation-2
branch
from
7af1d75
to
21358eb
Compare
| defines: { | ||
| NUM_DIMS: settings.dimensions, | ||
| NUM_CHANNELS: settings.numChannels, | ||
| SAMEPLER_WIDTH: TEXTURE_WIDTH |
There was a problem hiding this comment.
| SAMEPLER_WIDTH: TEXTURE_WIDTH | |
| SAMPLER_WIDTH: TEXTURE_WIDTH |
| } | ||
|
|
||
| dimensions: 1 | 2; | ||
| numChannels: 1 | 2 | 3; |
There was a problem hiding this comment.
Could we make the Aggregator class generic on the number of channels? That way we could catch out of bounds access at compile time, e.g.
type 3Channel = 0 | 1 | 2;
new GPUAggregator<3Channel>(...)
getResult(channel: Channels)| #endif | ||
|
|
||
| gl_Position = vec4(0., 0., 0., 1.); | ||
| gl_PointSize = 2.0; |
There was a problem hiding this comment.
Why is this 2, rather than 1 like in the bin-sorter?
There was a problem hiding this comment.
Oh I see the comment in the fragment shader now. Might be worth a small comment here also
| uniforms: { | ||
| // Passed in as uniform because 1) there is no GLSL symbol for NaN 2) any expression that exploits undefined behavior to produces NaN | ||
| // will subject to platform differences and shader optimization | ||
| naN: NaN |
There was a problem hiding this comment.
This looks risky, have we used this technique before?
| const MAX_FLOAT32 = 3e38; | ||
| const EMPTY_MASKS = {SUM: 0, MEAN: 0, MIN: 0, MAX: 0}; | ||
|
|
||
| export const TEXTURE_WIDTH = 1024; |
There was a problem hiding this comment.
Would it be worth exposing as a prop?
| } | ||
|
|
||
| function createModel(device: Device, settings: GPUAggregatorSettings): Model { | ||
| let userVs = settings.vs; |
There was a problem hiding this comment.
Could we provide a default, that just returns a constant, to avoid a shader compilation error?
| {name: 'income', format: 'float32', stepMode: 'vertex'}, | ||
| {name: 'education', format: 'float32', stepMode: 'vertex'} | ||
| ], | ||
| vs: ` |
There was a problem hiding this comment.
Really like how we can just supply aggregation snippets! 😊
For #7457
Change List
GPUAggregator