#124 comments addressed

UnfoldedInc · May 4, 2020 · 35be8bc · 35be8bc
1 parent cbe4516
commit 35be8bc
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 65 deletions.
diff --git a/cpp/examples/deck.gl/flight-paths.cc b/cpp/examples/deck.gl/flight-paths.cc
@@ -43,33 +43,26 @@ auto createViewState(double bearing) -> std::shared_ptr<ViewState> {
 }
 
 auto createLineLayer(const std::string &dataPath) -> std::shared_ptr<LineLayer::Props> {
-  auto lineLayerProps = std::make_shared<LineLayer::Props>();
-  lineLayerProps->id = "flight-paths";
-  lineLayerProps->opacity = 0.8f;
-  lineLayerProps->getSourcePosition = [](const Row &row) -> mathgl::Vector3<float> {
-    return row.getVector3<float>("start");
-  };
-  lineLayerProps->getTargetPosition = [](const Row &row) -> mathgl::Vector3<float> {
-    return row.getVector3<float>("end");
-  };
-  lineLayerProps->getColor = [](const Row &row) -> mathgl::Vector4<float> {
-    float z = row.getVector3<float>("start").z;
-    float r = z / 10000.0f;
+  auto props = std::make_shared<LineLayer::Props>();
+  props->id = "flight-paths";
+  props->opacity = 0.8f;
+  props->getSourcePosition = [](const Row &row) { return row.getVector3<float>("start"); };
+  props->getTargetPosition = [](const Row &row) { return row.getVector3<float>("end"); };
+  props->getColor = [](const Row &row) -> mathgl::Vector4<float> {
+    float r = row.getVector3<float>("start").z / 10000.0f;
     return {255.0f * (1.0f - r * 2.0f), 128.0f * r, 255.0f * r, 255.0f * (1.0f - r)};
   };
-  lineLayerProps->getWidth = [](const Row &row) -> float { return 3.0f; };
-  lineLayerProps->data = jsonLoader.loadTable(fileSystem->OpenInputStream(dataPath).ValueOrDie());
+  props->getWidth = [](const Row &row) { return 3.0f; };
+  props->data = jsonLoader.loadTable(fileSystem->OpenInputStream(dataPath).ValueOrDie());
 
-  return lineLayerProps;
+  return props;
 }
 
 auto createScatterplotLayer(const std::string &dataPath) -> std::shared_ptr<ScatterplotLayer::Props> {
-  auto scatterplotLayerProps = std::make_shared<ScatterplotLayer::Props>();
-  scatterplotLayerProps->id = "airports";
-  scatterplotLayerProps->getPosition = [](const Row &row) -> mathgl::Vector3<float> {
-    return row.getVector3<float>("coordinates");
-  };
-  scatterplotLayerProps->getRadius = [](const Row &row) -> float {
+  auto props = std::make_shared<ScatterplotLayer::Props>();
+  props->id = "airports";
+  props->getPosition = [](const Row &row) { return row.getVector3<float>("coordinates"); };
+  props->getRadius = [](const Row &row) -> float {
     auto type = row.getString("type");
     if (type == "major") {
       return 100.0f;
@@ -79,13 +72,15 @@ auto createScatterplotLayer(const std::string &dataPath) -> std::shared_ptr<Scat
       return 60.0f;
     }
   };
-  scatterplotLayerProps->getFillColor = [](const Row &row) -> mathgl::Vector4<float> {
-    return mathgl::Vector4<float>{255.0f, 144.0f, 0.0f, 255.0f};
-  };
-  scatterplotLayerProps->radiusScale = 20.0f;
-  scatterplotLayerProps->data = jsonLoader.loadTable(fileSystem->OpenInputStream(dataPath).ValueOrDie());
+  props->getFillColor = [](const Row &row) { return mathgl::Vector4<float>{255.0f, 144.0f, 0.0f, 255.0f}; };
+  props->radiusScale = 20.0f;
+  props->stroked = true;
+  props->getLineWidth = [](const Row &row) { return 5.0f; };
+  props->getLineColor = [](const Row &row) { return mathgl::Vector4<float>{255.0f, 0.0f, 0.0f, 255.0f}; };
+
+  props->data = jsonLoader.loadTable(fileSystem->OpenInputStream(dataPath).ValueOrDie());
 
-  return scatterplotLayerProps;
+  return props;
 }
 
 int main(int argc, const char *argv[]) {

diff --git a/cpp/modules/deck.gl/layers/src/line-layer/line-layer.cpp b/cpp/modules/deck.gl/layers/src/line-layer/line-layer.cpp
@@ -75,34 +75,46 @@ void LineLayer::initializeState() {
   auto getWidth = std::bind(&LineLayer::getWidthData, this, std::placeholders::_1);
   this->attributeManager->add(garrow::ColumnBuilder{width, getWidth});
 
-  // TODO(ilija@unfolded.ai): Where should we initialize models?
   this->models = {this->_getModel(this->context->device)};
+  this->_layerUniforms = std::make_shared<garrow::Array>(this->context->device);
 }
 
 void LineLayer::updateState(const Layer::ChangeFlags& changeFlags, const Layer::Props* oldProps) {
-  // super::updateState(props, oldProps, changeFlags);
-
-  // if (changeFlags.extensionsChanged) {
-  //   const {gl} = this->context;
-  //   if (this->state->model) {
-  //     this->state->model;
-  //   }
-  //   this->setState({model: this->_getModel(gl)});
-  //   this->getAttributeManager().invalidateAll();
-  // }
+  super::updateState(changeFlags, oldProps);
+
+  auto props = std::dynamic_pointer_cast<LineLayer::Props>(this->props());
+  float widthMultiplier = props->widthUnits == "pixels" ? this->context->viewport->metersPerPixel() : 1.0;
+  LineLayerUniforms layerUniforms;
+  layerUniforms.opacity = props->opacity;
+  layerUniforms.widthScale = props->widthScale * widthMultiplier;
+  layerUniforms.widthMinPixels = props->widthMinPixels;
+  layerUniforms.widthMaxPixels = props->widthMaxPixels;
+
+  this->_layerUniforms->setData(&layerUniforms, 1, wgpu::BufferUsage::Uniform);
+
+  /*
+  super::updateState(props, oldProps, changeFlags);
+
+  if (changeFlags.extensionsChanged) {
+    const {gl} = this->context;
+    if (this->state->model) {
+      this->state->model;
+    }
+    this->setState({model: this->_getModel(gl)});
+    this->getAttributeManager().invalidateAll();
+  }
+  */
 }
 
 void LineLayer::finalizeState() {}
 
 void LineLayer::drawState(wgpu::RenderPassEncoder pass) {
-  auto props = std::dynamic_pointer_cast<LineLayer::Props>(this->props());
-  float widthMultiplier = props->widthUnits == "pixels" ? this->context->viewport->metersPerPixel() : 1.0;
-  auto widthScale = props->widthScale * widthMultiplier;
-  LineLayerUniforms layerUniforms{props->opacity, widthScale, props->widthMinPixels, props->widthMaxPixels};
+  // TODO(ilija@unfolded.ai): Remove. updateState currently doesn't seem to be called when viewport changes
+  this->updateState(Layer::ChangeFlags{}, nullptr);
+
   for (auto const& model : this->getModels()) {
     // Layer uniforms are currently bound to index 1
-    model->setUniforms(
-        std::make_shared<garrow::Array>(this->context->device, &layerUniforms, 1, wgpu::BufferUsage::Uniform), 1);
+    model->setUniforms(this->_layerUniforms, 1);
     model->draw(pass);
   }
 }

diff --git a/cpp/modules/deck.gl/layers/src/line-layer/line-layer.h b/cpp/modules/deck.gl/layers/src/line-layer/line-layer.h
@@ -29,12 +29,15 @@
 #include <string>
 
 #include "deck.gl/core.h"
+#include "luma.gl/garrow.h"
 #include "luma.gl/webgpu.h"
 
 namespace deckgl {
 
 class LineLayer : public Layer {
  public:
+  using super = Layer;
+
   class Props;
   explicit LineLayer(std::shared_ptr<LineLayer::Props> props) : Layer{std::dynamic_pointer_cast<Layer::Props>(props)} {}
   auto props() { return std::dynamic_pointer_cast<Layer::Props>(this->_props); }
@@ -54,6 +57,8 @@ class LineLayer : public Layer {
 
  private:
   auto _getModel(wgpu::Device) -> std::shared_ptr<lumagl::Model>;
+
+  std::shared_ptr<lumagl::garrow::Array> _layerUniforms;
 };
 
 class LineLayer::Props : public Layer::Props {

diff --git a/cpp/modules/deck.gl/layers/src/scatterplot-layer/scatterplot-layer.cpp b/cpp/modules/deck.gl/layers/src/scatterplot-layer/scatterplot-layer.cpp
@@ -22,7 +22,6 @@
 
 #include "./scatterplot-layer-fragment.glsl.h"
 #include "./scatterplot-layer-vertex.glsl.h"
-#include "deck.gl/core.h"
 
 using namespace deckgl;
 using namespace lumagl;
@@ -115,11 +114,29 @@ void ScatterplotLayer::initializeState() {
   auto getLineWidth = std::bind(&ScatterplotLayer::getLineWidthData, this, std::placeholders::_1);
   this->attributeManager->add(garrow::ColumnBuilder{lineWidth, getLineWidth});
 
-  // TODO(ilija@unfolded.ai): Where should we initialize models?
   this->models = {this->_getModel(this->context->device)};
+  this->_layerUniforms = std::make_shared<garrow::Array>(this->context->device);
 }
 
 void ScatterplotLayer::updateState(const Layer::ChangeFlags& changeFlags, const Layer::Props* oldProps) {
+  super::updateState(changeFlags, oldProps);
+
+  auto props = std::dynamic_pointer_cast<ScatterplotLayer::Props>(this->props());
+  float widthMultiplier = props->lineWidthUnits == "pixels" ? this->context->viewport->metersPerPixel() : 1.0;
+
+  ScatterplotLayerUniforms uniforms;
+  uniforms.opacity = props->opacity;
+  uniforms.radiusScale = props->radiusScale;
+  uniforms.radiusMinPixels = props->radiusMinPixels;
+  uniforms.radiusMaxPixels = props->radiusMaxPixels;
+  uniforms.lineWidthScale = props->lineWidthScale * widthMultiplier;
+  uniforms.lineWidthMinPixels = props->lineWidthMinPixels;
+  uniforms.lineWidthMaxPixels = props->lineWidthMaxPixels;
+  uniforms.stroked = props->stroked ? 1.0f : 0.0f;
+  uniforms.filled = props->filled;
+
+  this->_layerUniforms->setData(&uniforms, 1, wgpu::BufferUsage::Uniform);
+
   /*
   super.updateState({props, oldProps, changeFlags});
   if (changeFlags.extensionsChanged) {
@@ -136,18 +153,12 @@ void ScatterplotLayer::updateState(const Layer::ChangeFlags& changeFlags, const
 void ScatterplotLayer::finalizeState() {}
 
 void ScatterplotLayer::drawState(wgpu::RenderPassEncoder pass) {
-  auto props = std::dynamic_pointer_cast<ScatterplotLayer::Props>(this->props());
-  float widthMultiplier = props->lineWidthUnits == "pixels" ? this->context->viewport->metersPerPixel() : 1.0;
-  auto lineWidthScale = props->lineWidthScale * widthMultiplier;
+  // TODO(ilija@unfolded.ai): Remove. updateState currently doesn't seem to be called when viewport changes
+  this->updateState(Layer::ChangeFlags{}, nullptr);
 
-  ScatterplotLayerUniforms layerUniforms{
-      props->opacity, props->radiusScale,        props->radiusMinPixels,    props->radiusMaxPixels,
-      lineWidthScale, props->lineWidthMinPixels, props->lineWidthMaxPixels, props->stroked ? 1.0f : 0.0f,
-      props->filled};
   for (auto const& model : this->getModels()) {
     // Layer uniforms are currently bound to index 1
-    model->setUniforms(
-        std::make_shared<garrow::Array>(this->context->device, &layerUniforms, 1, wgpu::BufferUsage::Uniform), 1);
+    model->setUniforms(this->_layerUniforms, 1);
     model->draw(pass);
   }
 }
@@ -218,6 +229,9 @@ auto ScatterplotLayer::_getModel(wgpu::Device device) -> std::shared_ptr<lumagl:
       vs, fs, attributeSchema, instancedAttributeSchema, uniforms, wgpu::PrimitiveTopology::TriangleStrip};
   auto model = std::make_shared<lumagl::Model>(device, modelOptions);
 
+  // a square that minimally cover the unit circle
+  // const positions = [ -1, -1, 0, -1, 1, 0, 1, 1, 0, 1, -1, 0 ];
+
   // A square that minimally covers the unit circle
   //
   //  (-1, -1)_------------_(1, -1)

diff --git a/cpp/modules/deck.gl/layers/src/scatterplot-layer/scatterplot-layer.h b/cpp/modules/deck.gl/layers/src/scatterplot-layer/scatterplot-layer.h
@@ -26,11 +26,15 @@
 #include <string>
 
 #include "deck.gl/core.h"
+#include "luma.gl/garrow.h"
+#include "luma.gl/webgpu.h"
 
 namespace deckgl {
 
 class ScatterplotLayer : public Layer {
  public:
+  using super = Layer;
+
   class Props;
   explicit ScatterplotLayer(std::shared_ptr<ScatterplotLayer::Props> props)
       : Layer{std::dynamic_pointer_cast<Layer::Props>(props)} {}
@@ -52,6 +56,8 @@ class ScatterplotLayer : public Layer {
 
  private:
   auto _getModel(wgpu::Device device) -> std::shared_ptr<lumagl::Model>;
+
+  std::shared_ptr<lumagl::garrow::Array> _layerUniforms;
 };
 
 class ScatterplotLayer::Props : public Layer::Props {

diff --git a/cpp/modules/luma.gl/garrow/src/array.cc b/cpp/modules/luma.gl/garrow/src/array.cc
@@ -43,9 +43,9 @@ void Array::setData(const std::shared_ptr<arrow::Array>& data, wgpu::BufferUsage
   auto vertexFormat = vertexFormatOptional.value();
   auto vertexSize = getVertexFormatSize(vertexFormat);
 
-  if (!this->_buffer || data->length() != this->_length) {
-    auto byteLength = vertexSize * data->length();
-    this->_buffer = this->_createBuffer(this->_device, byteLength, usage);
+  auto bufferByteSize = vertexSize * data->length();
+  if (!this->_buffer || bufferByteSize != this->_bufferByteSize) {
+    this->_buffer = this->_createBuffer(this->_device, bufferByteSize, usage);
   }
 
   // TODO(ilija@unfolded.ai): Handle arrays with null values correctly
@@ -73,6 +73,7 @@ void Array::setData(const std::shared_ptr<arrow::Array>& data, wgpu::BufferUsage
   }
 
   this->_length = data->length();
+  this->_bufferByteSize = bufferByteSize;
 }
 
 auto Array::_createBuffer(wgpu::Device device, uint64_t size, wgpu::BufferUsage usage) -> wgpu::Buffer {

diff --git a/cpp/modules/luma.gl/garrow/src/array.h b/cpp/modules/luma.gl/garrow/src/array.h
@@ -41,12 +41,10 @@ class Array {
   template <typename T>
   Array(wgpu::Device device, const std::vector<T>& data, wgpu::BufferUsage usage) : Array{device} {
     this->setData(data.data(), data.size(), usage);
-    this->_length = data.size();
   }
   template <typename T>
   Array(wgpu::Device device, const T* data, size_t length, wgpu::BufferUsage usage) : Array{device} {
     this->setData(data, length, usage);
-    this->_length = length;
   }
   ~Array();
 
@@ -55,16 +53,18 @@ class Array {
 
   /* Arrow non-compliant API */
 
-  // TODO(ilija@unfolded.ai): Is array/buffer mutability something we'll be making use of?
+  // TODO(ilija@unfolded.ai): Arrays are not mutable in Arrow. Revisit once ArrayData is in place
   void setData(const std::shared_ptr<arrow::Array>& data, wgpu::BufferUsage usage);
   template <typename T>
   void setData(const T* data, size_t length, wgpu::BufferUsage usage) {
-    auto bufferSize = sizeof(T) * length;
-    if (!this->_buffer || length != this->_length) {
-      this->_buffer = this->_createBuffer(this->_device, bufferSize, usage);
+    auto bufferByteSize = sizeof(T) * length;
+    if (!this->_buffer || bufferByteSize != this->_bufferByteSize) {
+      this->_buffer = this->_createBuffer(this->_device, bufferByteSize, usage);
     }
 
-    this->_buffer.SetSubData(0, bufferSize, data);
+    this->_buffer.SetSubData(0, bufferByteSize, data);
+    this->_length = length;
+    this->_bufferByteSize = bufferByteSize;
   }
 
   /// \brief Returns the backing buffer that this array manages.
@@ -76,6 +76,7 @@ class Array {
   wgpu::Device _device;
   wgpu::Buffer _buffer{nullptr};
   int64_t _length{0};
+  uint64_t _bufferByteSize{0};
 };
 
 }  // namespace garrow