Skip to content

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Sep 19, 2016

We expose the image resolution related GD functionality to userland
by introducing imagesetresolution(), imageresolutionx() and
imageresolutiony() as simple wrappers.

@nikic
Copy link
Member

nikic commented Sep 19, 2016

Not knowing anything about GD conventions, might it be preferable to have a imageresolution() instead of imageresolution{x,y}()? The function could, for example, return an (array) pair.

@cmb69
Copy link
Member Author

cmb69 commented Sep 19, 2016

@nikic AFAIK, the only precendence for getting a horizontal/vertical pair in GD is imagesx()/imagesy(). However, there is imagecolorsforindex() which returns an associative array of channel values. It seems, we're free to choose. :-)

Thinking about it, I tend to prefer a single function now – it'll be probably not often used, anyway. At least currently libgd doesn't do anything with the image resolution except for saving and loading it for PNG and JPEG images, so being able to set the resolution is presumably more important.

@krakjoe
Copy link
Member

krakjoe commented Oct 17, 2016

@cmb69 Can we get an update on your intentions here please ?

@cmb69
Copy link
Member Author

cmb69 commented Oct 17, 2016

@krakjoe Thanks for the reminder! I'll update the PR today or tomorrow.

We expose the image resolution related GD functionality to userland
by introducing `imageresolution()` as getter/setter. Given only the
image argument, it returns the current resolution as indexed array.
Given only a second argument, it sets the horizontal and vertical
resolution to this value. Given three arguments, it sets the horizontal
and vertical resolution to the given arguments, respectively.
@cmb69
Copy link
Member Author

cmb69 commented Oct 18, 2016

I've rewritten the PR to use a single function imageresolution() as getter and setter in the style of imageinterlace(). The return value of the getter is now a pair (i.e. an indexed array of the respective values).

Set and get image resolution of PNG images
--SKIPIF--
<?php
if (!extension_loaded('gd')) die('skip gd extension not available');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first test has a skip for supported image type, this one doesn't. Is there a reason for that ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PNG is always supported; JPEG only when requested during build time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's perfectly sensible. I wasn't sure, and did look for other tests checking for png, did find only one. Must be anomalous (maybe fix that when you have a moment).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 371f412.

@php-pulls php-pulls merged commit 209d422 into php:master Oct 20, 2016
php-pulls pushed a commit that referenced this pull request Oct 20, 2016
dstogov added a commit to zendtech/php-src that referenced this pull request Oct 21, 2016
* master: (24 commits)
  Turn IS_TYPE_COLLECTABLE zval flag into GC_COLLECTABLE zend_refcounted flag. This simplifies checks and allows reset this flag for "acyclic" arrays and objects.
  Fixed bug #73360 Unable to work in root with unicode chars
  Add tests for PDO::getAvailableDrivers
  Remove DBDO-specific field
  Save some more calls to strlen() on the CURLFile helper methods
  Revert "Fix test, this is kinda ugly, but at least for me on Windows there seems to be some messed up line endings"
  Save a call to strlen(), since we can figure out the length of this constant value with sizeof() at compile time
  Fix test, this is kinda ugly, but at least for me on Windows there seems to be some messed up line endings
  Fix bug #71241: array_replace_recursive mutates ref params
  Do not overwrite config.nice.bat if --with-config-profile is used on Windows
  Update UPGRADING wrt. imageresolution()
  Fixed test to accept MYSQLI_OPT_READ_TIMEOUT
  remove redundant includes
  fix Windows compilation
  Add php_random_int internal API
  Fix compiler warnings, always cast to zend_long from sqlite3_int64 when converting to a zval
  Ignore the return value of sqlite3->busyTimeout() if their "API Armor" is not enabled.
  news entry for pr php#2135
  news entry for pr php#2152
  news entry for #pr 2152
  ...
@cmb69 cmb69 deleted the imageresolution branch January 6, 2017 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants