Skip to content

Commit

Permalink
Render PNGs more efficiently
Browse files Browse the repository at this point in the history
Use file redirection in the OS shell to render PNG file rather
than reading them into memory. This also makes the rendering
an atomic operation from the application's point of view, so
perhaps will fix the issue with returning incomplete PNGs to the
client.

Bug: T219335
  • Loading branch information
samwilson committed May 1, 2019
1 parent 85cee92 commit c6783da
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 16 deletions.
20 changes: 11 additions & 9 deletions src/Controller/ApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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,
Expand All @@ -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']);
}
Expand Down
19 changes: 12 additions & 7 deletions src/Service/Renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

0 comments on commit c6783da

Please sign in to comment.