o Introduce the concept of a StaticTransformer: this is a transformer

that is applied at the beginning and permanently modifies the internal
  pixel mapping without the need for function calls at runtime.
o This fixes the confusing case that a FrameCanvas does not by default
  get the features of a CanvasTransformer.
o The old transformer interface is kept for backwards compatibility.
  (we might think of having that simplified with a callback function
   or something).
o The new function call is ApplyStaticTransformer(); the old methods
  SetTransformer() and transformer() are still supported but clearly
  marked deprecated.
o This is in preparation to provide different multiplexing (for outdoor
  panels) and have an easier way to provide a transformer in Python.

examples-api-use/demo-main.cc

@@ -65,7 +65,7 @@ public:
         g = 255 - c;
         b = c;
       }
-      matrix_->transformer()->Transform(off_screen_canvas_)->Fill(r, g, b);
+      off_screen_canvas_->Fill(r, g, b);
       off_screen_canvas_ = matrix_->SwapOnVSync(off_screen_canvas_);
     }
   }
@@ -279,10 +279,8 @@ public:
   }
 
   void Run() {
-    const int screen_height = matrix_->transformer()->Transform(offscreen_)
-      ->height();
-    const int screen_width = matrix_->transformer()->Transform(offscreen_)
-      ->width();
+    const int screen_height = offscreen_->height();
+    const int screen_width = offscreen_->width();
     while (running() && !interrupt_received) {
       {
         MutexLock l(&mutex_new_image_);
@@ -300,8 +298,7 @@ public:
         for (int y = 0; y < screen_height; ++y) {
           const Pixel &p = current_image_.getPixel(
             (horizontal_position_ + x) % current_image_.width, y);
-          matrix_->transformer()->Transform(offscreen_)->SetPixel(
-            x, y, p.red, p.green, p.blue);
+          offscreen_->SetPixel(x, y, p.red, p.green, p.blue);
         }
       }
       offscreen_ = matrix_->SwapOnVSync(offscreen_);
@@ -1148,16 +1145,13 @@ int main(int argc, char *argv[]) {
   if (matrix == NULL)
     return 1;
 
-  LinkedTransformer *transformer = new LinkedTransformer();
-  matrix->SetTransformer(transformer);
-
   if (large_display) {
     // Mapping the coordinates of a 32x128 display mapped to a square of 64x64
-    transformer->AddTransformer(new LargeSquare64x64Transformer());
+    matrix->ApplyStaticTransformer(LargeSquare64x64Transformer());
   }
 
   if (rotation > 0) {
-    transformer->AddTransformer(new RotateTransformer(rotation));
+    matrix->ApplyStaticTransformer(RotateTransformer(rotation));
   }
 
   Canvas *canvas = matrix;
@@ -1252,8 +1246,5 @@ int main(int argc, char *argv[]) {
   delete image_gen;
   delete canvas;
 
-  transformer->DeleteTransformers();
-  delete transformer;
-
   return 0;
 }

include/led-matrix.h

@@ -214,11 +214,36 @@ public:
   // 28Hz animation, nicely locked to the frame-rate).
   FrameCanvas *SwapOnVSync(FrameCanvas *other, unsigned framerate_fraction = 1);
 
-  // Set image transformer that maps the logical canvas we provide to the
-  // physical canvas (e.g. panel mapping, rotation ...).
-  // Does _not_ take ownership of the transformer.
-  void SetTransformer(CanvasTransformer *transformer);
-  inline CanvasTransformer *transformer() { return transformer_; }
+  // Set image transformer that maps the logical canvas coordinates to the
+  // physical canvas coordinates.
+  // This preprocesses the transformation for static pixel mapping once.
+  //
+  // (In the rate case that you have transformers that dynamically change
+  //  their behavior at runtime or do transformations on the color, you have to
+  //  manually use them to wrap canvases.)
+  void ApplyStaticTransformer(const CanvasTransformer &transformer);
+
+  // Don't use this function anymore, use ApplyStaticTransformer() instead.
+  // See demo-main.cc how.
+  //
+  // This used to somewhat work with dynamic tranformations, but it
+  // was confusing as that didn't apply to FrameCanvases as well.
+  // If you have static transformations that can be done at program start
+  // (such as rotation or creating your particular pysical display mapping),
+  // use ApplyStaticTransformer().
+  // If you use the Transformer concept to modify writes to canvases on-the-fly,
+  // use them directly as such.
+  //
+  // DO NOT USE. WILL BE REMOVED.
+  void SetTransformer(CanvasTransformer *t) __attribute__((deprecated)) {
+    transformer_ = t;
+    if (t) ApplyStaticTransformer(*t);
+  }
+
+  // DO NOT USE. WILL BE REMOVED.
+  CanvasTransformer *transformer() __attribute__((deprecated)) {
+    return transformer_;
+  }
 
   // -- Canvas interface. These write to the active FrameCanvas
   // (see documentation in canvas.h)
@@ -240,9 +265,9 @@ private:
 
   GPIO *io_;
   Mutex active_frame_sync_;
+  CanvasTransformer *transformer_;  // deprecated. To be removed.
   UpdateThread *updater_;
   std::vector<FrameCanvas*> created_frames_;
-  CanvasTransformer *transformer_;
   internal::PixelMapper *shared_pixel_mapper_;
 };
 

include/transformer.h

@@ -32,7 +32,7 @@ public:
 
   void SetAngle(int angle);
   inline int angle() { return angle_; }
-  
+
   virtual Canvas *Transform(Canvas *output);
 
 private:
@@ -49,7 +49,7 @@ private:
 class LinkedTransformer : public CanvasTransformer {
 public:
   typedef std::vector<CanvasTransformer*> List;
-  
+
   LinkedTransformer() {}
   LinkedTransformer(List transformer_list) : list_(transformer_list) {}
 

lib/framebuffer.cc

@@ -54,7 +54,7 @@ static PinPulser *sOutputEnablePulser = NULL;
 #  define SUB_PANELS_ 2
 #endif
 
-inline PixelDesignator *PixelMapper::get(int x, int y) {
+PixelDesignator *PixelMapper::get(int x, int y) {
   if (x < 0 || y < 0 || x >= width_ || y >= height_)
     return NULL;
   return buffer_ + (y*width_) + x;

lib/led-matrix.cc

@@ -30,14 +30,6 @@
 #include "framebuffer-internal.h"
 
 namespace rgb_matrix {
-
-namespace {
-class NullTransformer : public CanvasTransformer {
-public:
-  virtual Canvas *Transform(Canvas *output) { return output; }
-};
-}  // anonymous namespace
-
 // Pump pixels to screen. Needs to be high priority real-time because jitter
 class RGBMatrix::UpdateThread : public Thread {
 public:
@@ -167,7 +159,6 @@ RGBMatrix::Options::Options()
 RGBMatrix::RGBMatrix(GPIO *io, const Options &options)
   : params_(options), io_(NULL), updater_(NULL), shared_pixel_mapper_(NULL) {
   assert(params_.Validate(NULL));
-  SetTransformer(NULL);
   active_ = CreateFrameCanvas();
   Clear();
   SetGPIO(io, true);
@@ -180,7 +171,6 @@ RGBMatrix::RGBMatrix(GPIO *io, int rows, int chained_displays,
   params_.chain_length = chained_displays;
   params_.parallel = parallel_displays;
   assert(params_.Validate(NULL));
-  SetTransformer(NULL);
   active_ = CreateFrameCanvas();
   Clear();
   SetGPIO(io, true);
@@ -258,15 +248,6 @@ FrameCanvas *RGBMatrix::SwapOnVSync(FrameCanvas *other,
   return previous;
 }
 
-void RGBMatrix::SetTransformer(CanvasTransformer *transformer) {
-  if (transformer == NULL) {
-    static NullTransformer null_transformer;   // global instance sufficient.
-    transformer_ = &null_transformer;
-  } else {
-    transformer_ = transformer;
-  }
-}
-
 bool RGBMatrix::SetPWMBits(uint8_t value) {
   const bool success = active_->framebuffer()->SetPWMBits(value);
   if (success) {
@@ -296,23 +277,93 @@ uint8_t RGBMatrix::brightness() {
 
 // -- Implementation of RGBMatrix Canvas: delegation to ContentBuffer
 int RGBMatrix::width() const {
-  return transformer_->Transform(active_)->width();
+  return active_->width();
 }
 
 int RGBMatrix::height() const {
-  return transformer_->Transform(active_)->height();
+  return active_->height();
 }
 
 void RGBMatrix::SetPixel(int x, int y, uint8_t red, uint8_t green, uint8_t blue) {
-  transformer_->Transform(active_)->SetPixel(x, y, red, green, blue);
+  active_->SetPixel(x, y, red, green, blue);
 }
 
 void RGBMatrix::Clear() {
-  transformer_->Transform(active_)->Clear();
+  active_->Clear();
 }
 
 void RGBMatrix::Fill(uint8_t red, uint8_t green, uint8_t blue) {
-  transformer_->Transform(active_)->Fill(red, green, blue);
+  active_->Fill(red, green, blue);
+}
+
+namespace {
+// A pixel mapper
+class PixelMapExtractionCanvas : public Canvas {
+public:
+  PixelMapExtractionCanvas(internal::PixelMapper *old_mapper)
+    : old_mapper_(old_mapper), new_mapper_(NULL) {}
+
+  virtual int width() const { return old_mapper_->width(); }
+  virtual int height() const { return old_mapper_->height(); }
+
+  void SetNewMapper(internal::PixelMapper *new_mapper) {
+    new_mapper_ = new_mapper;
+  }
+  void SetNewLocation(int x, int y) {
+    x_new = x;
+    y_new = y;
+  }
+  virtual void SetPixel(int x, int y, uint8_t r, uint8_t g, uint8_t b) {
+    const internal::PixelDesignator *orig_designator = old_mapper_->get(x, y);
+    if (orig_designator && new_mapper_) {
+      // Tell the new mapper at the new location what after the mapping was
+      // at the old location.
+      *new_mapper_->get(x_new, y_new) = *orig_designator;
+    }
+  }
+
+  virtual void Clear() {}
+  virtual void Fill(uint8_t red, uint8_t green, uint8_t blue) {}
+
+private:
+  internal::PixelMapper *const old_mapper_;
+  internal::PixelMapper *new_mapper_;
+  int x_new, y_new;
+};
+}  // anonymous namespace
+void RGBMatrix::ApplyStaticTransformer(const CanvasTransformer &transformer) {
+  using internal::PixelMapper;
+  assert(shared_pixel_mapper_);  // Not initialized yet ?
+  PixelMapExtractionCanvas extractor_canvas(shared_pixel_mapper_);
+
+  // These transformers traditionally only a non-const Transform()
+  // method, so that they can modify an instance variable keeping the delegate.
+  //
+  // We can't really change that now as we want to be backwards compatible.
+  //
+  // Having a const-reference as parameter to ApplyStaticTransformer() however
+  // is somewhat neat, so that it is possible to pass ad-hoc instances to
+  // ApplyStaticTransformer() (which are discarded after that all anyway).
+  //
+  // So we're slightly naughty here and cast the const away.
+  CanvasTransformer *non_const_transformer
+    = const_cast<CanvasTransformer*>(&transformer);
+  Canvas *mapped_canvas = non_const_transformer->Transform(&extractor_canvas);
+
+  const int new_width = mapped_canvas->width();
+  const int new_height = mapped_canvas->height();
+  PixelMapper *new_mapper = new PixelMapper(new_width, new_height);
+  extractor_canvas.SetNewMapper(new_mapper);
+  // Learn about the pixel mapping by going through all transformed pixels and
+  // build new PixelDesignator map.
+  for (int y = 0; y < new_height; ++y) {
+    for (int x = 0; x < new_width; ++x) {
+      extractor_canvas.SetNewLocation(x, y);
+      mapped_canvas->SetPixel(x, y, 0, 0, 0); // force copy of designator.
+    }
+  }
+  delete shared_pixel_mapper_;
+  shared_pixel_mapper_ = new_mapper;
 }
 
 // FrameCanvas implementation of Canvas

utils/led-image-viewer.cc

@@ -37,7 +37,6 @@ using rgb_matrix::GPIO;
 using rgb_matrix::Canvas;
 using rgb_matrix::FrameCanvas;
 using rgb_matrix::RGBMatrix;
-using rgb_matrix::CanvasTransformer;
 
 volatile bool interrupt_received = false;
 static void InterruptHandler(int signo) {
@@ -65,29 +64,22 @@ namespace {
 class PreprocessedFrame {
 public:
   PreprocessedFrame(const Magick::Image &img, bool do_center,
-                    CanvasTransformer *transformer,
                     rgb_matrix::FrameCanvas *output)
     : canvas_(output) {
     int delay_time = img.animationDelay();  // in 1/100s of a second.
     if (delay_time < 1) delay_time = 1;
     delay_millis_ = delay_time * 10;
 
-    Canvas *const transformed_draw_canvas = transformer->Transform(output);
-    int x_offset = (do_center
-                    ? (transformed_draw_canvas->width() - img.columns()) / 2
-                    : 0);
-    int y_offset = (do_center
-                    ? (transformed_draw_canvas->height() - img.rows()) / 2
-                    : 0);
+    const int x_offset = do_center ? (output->width() - img.columns()) / 2 : 0;
+    const int y_offset = do_center ? (output->height() - img.rows()) / 2 : 0;
     for (size_t y = 0; y < img.rows(); ++y) {
       for (size_t x = 0; x < img.columns(); ++x) {
         const Magick::Color &c = img.pixelColor(x, y);
         if (c.alphaQuantum() < 256) {
-          transformed_draw_canvas
-            ->SetPixel(x + x_offset, y + y_offset,
-                       ScaleQuantumToChar(c.redQuantum()),
-                       ScaleQuantumToChar(c.greenQuantum()),
-                       ScaleQuantumToChar(c.blueQuantum()));
+          output->SetPixel(x + x_offset, y + y_offset,
+                           ScaleQuantumToChar(c.redQuantum()),
+                           ScaleQuantumToChar(c.greenQuantum()),
+                           ScaleQuantumToChar(c.blueQuantum()));
         }
       }
     }
@@ -288,7 +280,7 @@ int main(int argc, char *argv[]) {
 
   if (large_display) {
     // Mapping the coordinates of a 32x128 display mapped to a square of 64x64
-    matrix->SetTransformer(new rgb_matrix::LargeSquare64x64Transformer());
+    matrix->ApplyStaticTransformer(rgb_matrix::LargeSquare64x64Transformer());
   }
 
   // These parameters are needed once we do scrolling.
@@ -309,12 +301,11 @@ int main(int argc, char *argv[]) {
       }
 
       PreprocessedList frames;
-      CanvasTransformer *const transformer = matrix->transformer();
       // Convert to preprocessed frames.
       for (size_t i = 0; i < image_sequence.size(); ++i) {
         FrameCanvas *canvas = matrix->CreateFrameCanvas();
         frames.push_back(new PreprocessedFrame(image_sequence[i], do_center,
-                                               transformer, canvas));
+                                               canvas));
       }
       // The 'animation delay' of a single image is the time to the next image.
       if (frames.size() == 1)