Merge pull request #99 from wikimedia/processing-T219335

Render PNGs more efficiently
wikimedia · Jul 26, 2019 · f80215e · f80215e
2 parents bfb590e + c6783da
commit f80215e
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 16 deletions.
diff --git a/src/Controller/ApiController.php b/src/Controller/ApiController.php
@@ -54,14 +54,16 @@ public function getFile(string $filename, string $lang): Response
     }
 
     /**
-     * Get a full filesystem path to a temporary PNG file.
+     * Get a full filesystem path to a temporary file.
      * @param string $filename The base SVG filename.
      * @param string $key The unique key to append to the filename.
+     * @param string $ext The file extension to use for the returned filename.
      * @return string
      */
-    protected function getTempPngFilename(string $filename, string $key): string
+    protected function getTempFilename(string $filename, string $key, string $ext): string
     {
-        return $this->cache->fullPath($filename.'_'.$key.'.png');
+        $name = pathinfo($filename, PATHINFO_FILENAME);
+        return $this->cache->fullPath($name.'_'.$key.'.'.$ext);
     }
 
     /**
@@ -85,14 +87,14 @@ public function requestFileWithTranslations(string $filename, string $lang, Requ
         // Write the modified SVG out to the filesystem, named with a unique key. This is necessary
         // both because multiple people could be translating the same file at the same time, and
         // also means we can use the same key for the rendered PNG file. The key is generated from
-        // the translation set so that the
+        // the translation set in order to not generate redundant cache files.
         $fileKey = md5(serialize($translations));
-        $tempPngFilename = $this->getTempPngFilename($filename, $fileKey);
-        $file->saveToPath($tempPngFilename);
+        $tempPngFilename = $this->getTempFilename($filename, $fileKey, 'png');
+        $tempSvgFilename = $this->getTempFilename($filename, $fileKey, 'svg');
+        $file->saveToPath($tempSvgFilename);
 
         // Render the modified SVG to PNG, and return it's URL.
-        $renderedPngContents = $this->svgRenderer->render($tempPngFilename, $lang);
-        file_put_contents($tempPngFilename, $renderedPngContents);
+        $this->svgRenderer->render($tempSvgFilename, $lang, $tempPngFilename);
         $relativeUrl = $this->generateUrl('api_file_translated', [
             'filename' => $filename,
             'key' => $fileKey,
@@ -113,7 +115,7 @@ public function requestFileWithTranslations(string $filename, string $lang, Requ
      */
     public function getFileWithTranslations(string $filename, string $lang, string $key, Request $request): Response
     {
-        $tempPngFilename = $this->getTempPngFilename($filename, $key);
+        $tempPngFilename = $this->getTempFilename($filename, $key, 'png');
         if (file_exists($tempPngFilename)) {
             return new BinaryFileResponse($tempPngFilename, 200, ['X-Accel-Buffering' => 'no']);
         }

diff --git a/src/Service/Renderer.php b/src/Service/Renderer.php
@@ -24,24 +24,29 @@ public function __construct(string $rsvgCommand)
     }
 
     /**
+     * Render a SVG file to PNG and either save a file or return the image.
      * @param string $file Full filesystem path to the SVG file to render.
      * @param string $lang Code of the language in which to render the image.
+     * @param string $outFile Full filesystem path to the file to write the PNG to.
      * @throws ProcessFailedException If the PNG conversion failed.
-     * @return string The PNG image contents.
+     * @return string The PNG image contents, or nothing if an $outFile was provided.
      */
-    public function render(string $file, string $lang) : string
+    public function render(string $file, string $lang, ?string $outFile = null) : string
     {
-        $process = new Process([$this->rsvgCommand, $file]);
+        // Construct the command, using variables that will be escaped when it's run.
+        $command = $this->rsvgCommand.' "$SVG"';
+        if ($outFile) {
+            // Redirect to output file if required.
+            $command .= ' > "$PNG"';
+        }
+        $process = Process::fromShellCommandline($command);
         if ('fallback' !== $lang) {
             // Set the LANG environment variable, which will be interpreted as the SVG
             // systemLanguage. If the fallback language is being requested, the OS's default will be
             // used instead (as is done in MediaWiki).
             $process->setEnv(['LANG' => $lang]);
         }
-        $process->run();
-        if (!$process->isSuccessful()) {
-            throw new ProcessFailedException($process);
-        }
+        $process->mustRun(null, ['SVG' => $file, 'PNG' => $outFile]);
         return $process->getOutput();
     }
 }