Auto orientation problem #47

Closed
alchemydigital opened this Issue Mar 15, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@alchemydigital

Not strictly a bug with Glide, but more a limitation of Intervention that affects Glide functionality. As mentioned in this issue Intervention can only read EXIF data if the image object is initialised with a file path (since it uses exif_read_data). Since Glide always passes in the file contents (at it is loading the image via Flysystem) this means that auto-orientation will never work as there will be no EXIF information for Intervention to use. Explicit orientation (i.e. rotate 90deg) is unaffected.

A quick and dirty fix that we have employed is to update the makeImage() method in League\Glide\Server\Server.php to write the source file to a temporary file in the system tmp/ folder and then pass that path to the API run method (with appropriate garbage collection to clear out the temporary file once it has been used):

$source = $this->source->read(
    $this->getSourcePath($request)
);

if ($source === false) {
    throw new FilesystemException(
        'Could not read the image `'.$this->getSourcePath($request).'`.'
    );
}

// Write the source to a temporary file
$tmp = tempnam(sys_get_temp_dir(), '');
$handle = fopen($tmp, "w");
fwrite($handle, $source);

try {
    $write = $this->cache->write(
        $this->getCachePath($request),
        // Pass the path to the temporary file instead of the source itself
        $this->api->run($request, $tmp)
    );
} catch (FileExistsException $exception) {
    // Cache file failed to write. Fail silently.
    // Remove the temporary file
    unlink($tmp);
    return $request;
}

// Remove the temporary file
unlink($tmp);

Happy to make a PR with this suggested change - even happier to see a more elegant solution!

@reinink

This comment has been minimized.

Show comment
Hide comment
@reinink

reinink Mar 16, 2015

Member

Hmm, that is NOT good. Thank you very much for finding this—that makes total sense. Let me think about how best to solve this...your temp file solution may be the only good option we have, but let me do some digging first.

Member

reinink commented Mar 16, 2015

Hmm, that is NOT good. Thank you very much for finding this—that makes total sense. Let me think about how best to solve this...your temp file solution may be the only good option we have, but let me do some digging first.

@renege

This comment has been minimized.

Show comment
Hide comment

renege commented Apr 12, 2015

Update?

@reinink

This comment has been minimized.

Show comment
Hide comment
@reinink

reinink Apr 20, 2015

Member

@renege Soon. Very sorry for the delay. See this and maybe this.

Member

reinink commented Apr 20, 2015

@renege Soon. Very sorry for the delay. See this and maybe this.

@alchemydigital

This comment has been minimized.

Show comment
Hide comment
@alchemydigital

alchemydigital Apr 20, 2015

It's also simple enough to do something like this and this whilst @reinink works through his backlog.

It's also simple enough to do something like this and this whilst @reinink works through his backlog.

@soee

This comment has been minimized.

Show comment
Hide comment
@soee

soee Apr 22, 2015

+1 for some fix for this

soee commented Apr 22, 2015

+1 for some fix for this

@reinink

This comment has been minimized.

Show comment
Hide comment
@reinink

reinink May 22, 2015

Member

This has been fixed (57a10a4) and tagged in 0.3.5.

Member

reinink commented May 22, 2015

This has been fixed (57a10a4) and tagged in 0.3.5.

@reinink reinink closed this Jun 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment