Per review:

* Use Light::Evaluated to initialize FillExtrusionUniforms + FillExtrusionPatternUniforms
* painter_fill_extrusion.cpp reindentation

4 files changed, 85 insertions, 44 deletions

diff --git a/src/mbgl/programs/fill_extrusion_program.cpp b/src/mbgl/programs/fill_extrusion_program.cpp
index 468d184978..36d7818b2a 100644
--- a/src/mbgl/programs/fill_extrusion_program.cpp
+++ b/src/mbgl/programs/fill_extrusion_program.cpp
@@ -3,6 +3,7 @@
 #include <mbgl/style/cross_faded_property_evaluator.hpp>
 #include <mbgl/tile/tile_id.hpp>
 #include <mbgl/map/transform_state.hpp>
+#include <mbgl/util/mat3.hpp>
 
 namespace mbgl {
 
@@ -10,6 +11,38 @@ using namespace style;
 
 static_assert(sizeof(FillExtrusionLayoutVertex) == 12, "expected FillExtrusionLayoutVertex size");
 
+std::array<float, 3> lightColor(const style::Light::Evaluated& light) {
+    const auto color = light.get<LightColor>();
+    return {{ color.r, color.g, color.b }};
+}
+
+std::array<float, 3> lightPosition(const style::Light::Evaluated& light, const TransformState& state) {
+    auto lightPos = light.get<LightPosition>().getCartesian();
+    mat3 lightMat;
+    matrix::identity(lightMat);
+    if (light.get<LightAnchor>() == LightAnchorType::Viewport) {
+        matrix::rotate(lightMat, lightMat, -state.getAngle());
+    }
+    matrix::transformMat3f(lightPos, lightPos, lightMat);
+    return lightPos;
+}
+
+float lightIntensity(const style::Light::Evaluated& light) {
+    return light.get<LightIntensity>();
+}
+
+FillExtrusionUniforms::Values
+FillExtrusionUniforms::values(mat4 matrix,
+                              const TransformState& state,
+                              const style::Light::Evaluated& light) {
+    return FillExtrusionUniforms::Values{
+        uniforms::u_matrix::Value{ matrix },
+        uniforms::u_lightcolor::Value{ lightColor(light) },
+        uniforms::u_lightpos::Value{ lightPosition(light, state) },
+        uniforms::u_lightintensity::Value{ lightIntensity(light) }
+    };
+}
+
 FillExtrusionPatternUniforms::Values
 FillExtrusionPatternUniforms::values(mat4 matrix,
                                      const SpriteAtlasElement& a,
@@ -18,9 +51,7 @@ FillExtrusionPatternUniforms::values(mat4 matrix,
                                      const UnwrappedTileID& tileID,
                                      const TransformState& state,
                                      const float heightFactor,
-                                     const std::array<float, 3>& lightColor,
-                                     const std::array<float, 3>& lightPos,
-                                     const float lightIntensity) {
+                                     const style::Light::Evaluated& light) {
     int32_t tileSizeAtNearestZoom = util::tileSize * state.zoomScale(state.getIntegerZoom() - tileID.canonical.z);
     int32_t pixelX = tileSizeAtNearestZoom * (tileID.canonical.x + tileID.wrap * state.zoomScale(tileID.canonical.z));
     int32_t pixelY = tileSizeAtNearestZoom * tileID.canonical.y;
@@ -41,9 +72,9 @@ FillExtrusionPatternUniforms::values(mat4 matrix,
         uniforms::u_pixel_coord_lower::Value{ std::array<float, 2>{{ float(pixelX & 0xFFFF), float(pixelY & 0xFFFF) }} },
         uniforms::u_tile_units_to_pixels::Value{ 1.0f / tileID.pixelsToTileUnits(1.0f, state.getIntegerZoom()) },
         uniforms::u_height_factor::Value{ heightFactor },
-        uniforms::u_lightcolor::Value{ lightColor },
-        uniforms::u_lightpos::Value{ lightPos },
-        uniforms::u_lightintensity::Value{ lightIntensity },
+        uniforms::u_lightcolor::Value{ lightColor(light) },
+        uniforms::u_lightpos::Value{ lightPosition(light, state) },
+        uniforms::u_lightintensity::Value{ lightIntensity(light) },
     };
 }
 
diff --git a/src/mbgl/programs/fill_extrusion_program.hpp b/src/mbgl/programs/fill_extrusion_program.hpp
index df6dc2fe5f..ed4ba6fe00 100644
--- a/src/mbgl/programs/fill_extrusion_program.hpp
+++ b/src/mbgl/programs/fill_extrusion_program.hpp
@@ -10,6 +10,7 @@
 #include <mbgl/util/size.hpp>
 #include <mbgl/style/layers/fill_extrusion_layer_properties.hpp>
 #include <mbgl/style/style.hpp>
+#include <mbgl/style/light.hpp>
 
 #include <string>
 
@@ -41,7 +42,11 @@ struct FillExtrusionUniforms : gl::Uniforms<
     uniforms::u_lightcolor,
     uniforms::u_lightpos,
     uniforms::u_lightintensity>
-{};
+{
+    static Values values(mat4,
+                         const TransformState&,
+                         const style::Light::Evaluated&);
+};
 
 struct FillExtrusionPatternUniforms : gl::Uniforms<
     uniforms::u_matrix,
@@ -63,16 +68,14 @@ struct FillExtrusionPatternUniforms : gl::Uniforms<
     uniforms::u_lightpos,
     uniforms::u_lightintensity>
 {
-    static Values values(mat4 matrix,
+    static Values values(mat4,
                          const SpriteAtlasElement&,
                          const SpriteAtlasElement&,
                          const style::Faded<std::string>&,
                          const UnwrappedTileID&,
                          const TransformState&,
                          const float,
-                         const std::array<float, 3>&,
-                         const std::array<float, 3>&,
-                         const float);
+                         const style::Light::Evaluated&);
 };
 
 class FillExtrusionProgram : public Program<
diff --git a/src/mbgl/renderer/painter.cpp b/src/mbgl/renderer/painter.cpp
index 4a62bd3973..70ff39cb18 100644
--- a/src/mbgl/renderer/painter.cpp
+++ b/src/mbgl/renderer/painter.cpp
@@ -358,7 +358,7 @@ void Painter::renderPass(PaintParameters& parameters,
                     uniforms::u_opacity::Value{
                         layer.as<FillExtrusionLayer>()
                             ->impl->paint.evaluated.get<FillExtrusionOpacity>() } },
-                extrusionTextureVertexBuffer, extrusionTextureTriangleIndexBuffer,
+                extrusionTextureVertexBuffer, quadTriangleIndexBuffer,
                 extrusionTextureSegments,
                 ExtrusionTextureProgram::PaintPropertyBinders{ properties, 0 }, properties,
                 state.getZoom());
diff --git a/src/mbgl/renderer/painter_fill_extrusion.cpp b/src/mbgl/renderer/painter_fill_extrusion.cpp
index 3ceda03c6d..71f5c24fe2 100644
--- a/src/mbgl/renderer/painter_fill_extrusion.cpp
+++ b/src/mbgl/renderer/painter_fill_extrusion.cpp
@@ -9,7 +9,6 @@
 #include <mbgl/programs/fill_extrusion_program.hpp>
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/convert.hpp>
-#include <mbgl/util/mat3.hpp>
 
 namespace mbgl {
 
@@ -27,19 +26,6 @@ void Painter::renderFillExtrusion(PaintParameters& parameters,
     }
 
     const auto light = style.light.evaluated;
-    const auto lightColor = light.get<LightColor>();
-    std::array<float, 3> color3f = { { lightColor.r, lightColor.g, lightColor.b } };
-    const auto lightAnchor = light.get<LightAnchor>();
-
-    auto lightPos = light.get<LightPosition>().getCartesian();
-    mat3 lightmat;
-    matrix::identity(lightmat);
-    if (lightAnchor == LightAnchorType::Viewport) {
-        matrix::rotate(lightmat, lightmat, -state.getAngle());
-    }
-    matrix::transformMat3f(lightPos, lightPos, lightmat);
-
-    const auto lightIntensity = light.get<LightIntensity>();
 
     if (!properties.get<FillExtrusionPattern>().from.empty()) {
         optional<SpriteAtlasElement> imagePosA =
@@ -54,29 +40,50 @@ void Painter::renderFillExtrusion(PaintParameters& parameters,
         spriteAtlas->bind(true, context, 0);
 
         parameters.programs.fillExtrusionPattern.draw(
-            context, gl::Triangles(), depthModeForSublayer(0, gl::DepthMode::ReadWrite),
-            gl::StencilMode::disabled(), colorModeForRenderPass(),
+            context,
+            gl::Triangles(),
+            depthModeForSublayer(0, gl::DepthMode::ReadWrite),
+            gl::StencilMode::disabled(),
+            colorModeForRenderPass(),
             FillExtrusionPatternUniforms::values(
                 tile.translatedMatrix(properties.get<FillExtrusionTranslate>(),
-                                      properties.get<FillExtrusionTranslateAnchor>(), state),
-                *imagePosA, *imagePosB, properties.get<FillExtrusionPattern>(), tile.id, state,
-                -std::pow(2, tile.id.canonical.z) / util::tileSize / 8.0f, color3f, lightPos,
-                lightIntensity),
-            *bucket.vertexBuffer, *bucket.indexBuffer, bucket.triangleSegments,
-            bucket.paintPropertyBinders.at(layer.getID()), properties, state.getZoom());
+                                      properties.get<FillExtrusionTranslateAnchor>(),
+                                      state),
+                *imagePosA,
+                *imagePosB,
+                properties.get<FillExtrusionPattern>(),
+                tile.id,
+                state,
+                -std::pow(2, tile.id.canonical.z) / util::tileSize / 8.0f,
+                light
+            ),
+            *bucket.vertexBuffer,
+            *bucket.indexBuffer,
+            bucket.triangleSegments,
+            bucket.paintPropertyBinders.at(layer.getID()),
+            properties,
+            state.getZoom());
 
     } else {
         parameters.programs.fillExtrusion.draw(
-            context, gl::Triangles(), depthModeForSublayer(0, gl::DepthMode::ReadWrite),
-            gl::StencilMode::disabled(), colorModeForRenderPass(),
-            FillExtrusionProgram::UniformValues{
-                uniforms::u_matrix::Value{
-                    tile.translatedMatrix(properties.get<FillExtrusionTranslate>(),
-                                          properties.get<FillExtrusionTranslateAnchor>(), state) },
-                uniforms::u_lightcolor::Value{ color3f }, uniforms::u_lightpos::Value{ lightPos },
-                uniforms::u_lightintensity::Value{ lightIntensity } },
-            *bucket.vertexBuffer, *bucket.indexBuffer, bucket.triangleSegments,
-            bucket.paintPropertyBinders.at(layer.getID()), properties, state.getZoom());
+            context,
+            gl::Triangles(),
+            depthModeForSublayer(0, gl::DepthMode::ReadWrite),
+            gl::StencilMode::disabled(),
+            colorModeForRenderPass(),
+            FillExtrusionUniforms::values(
+                tile.translatedMatrix(properties.get<FillExtrusionTranslate>(),
+                                      properties.get<FillExtrusionTranslateAnchor>(),
+                                      state),
+                state,
+                light
+            ),
+            *bucket.vertexBuffer,
+            *bucket.indexBuffer,
+            bucket.triangleSegments,
+            bucket.paintPropertyBinders.at(layer.getID()),
+            properties,
+            state.getZoom());
     };
 }