MapScript colorObj SWIG/PHP alignment and methods with alpha #5078
Conversation
|
Will check the test failures and update the PR tomorrow. |
ejn
force-pushed
the
mapscriptColorAlpha70
branch
2 times, most recently
from
bcc6027
to
d9f81a1
Compare
|
It looks like the failing tests after the updated PR are in gdal and the renderers, neither of which should be at all affected by this PR. @tbonfort Can you make sense of that? I've not got a working test environment here at the minute which isn't make life easier. |
|
no worries on the failed test, the remote wms server we're using must be down or changed... just wondering about the need to add setRGBA from fromHexWithAlpha, can't we just extend the current setRGB (with a default alpha=255) and fromHex and not add new signatures? |
|
@tbonfort Thanks for checking the test failure - I was reasonably sure it wasn't my fault! There is no fromHexWithAlpha - the existing fromHex (in the case of SWIG, it wasn't in PHP at all) just accepts either a 3- or 4-byte hex string. For toHex() and toHexWithAlpha() I guess toHex() - up until now only in SWIG - could be extended with an optional flag argument withAlpha (default false for backwards compatibility). Same with setRGB()/setRGBA(). I'm +/-0 on optional arguments or separate methods - I find a separate method more explicit, but optional arguments are probably cleaner. Not sure what the usual practice has been up to now in Mapscript? Either way I can update the PR if necessary. |
|
I'd prefer just setRGB, toHex and fromHex, with setRGB taking a default fourth alpha argument, fromHex being extended to parse #rrggbb and #rrggbbaa , and toHex spitting out #rrggbb or #rrggbbaa depending on wether opacity is full or not. We might also want to refactor the code at https://github.com/mapserver/mapserver/blob/branch-7-0/mapfile.c#L405 into a new function that could be called by mapscripts. |
ejn
force-pushed
the
mapscriptColorAlpha70
branch
from
99f742e
to
e2b2eb4
Compare
Adds tests related to Mapserver PR MapServer/MapServer#5078
This PR updates the documentation for colorObj in SWIG- and PHP- Mapscripts to reflect the alpha component, including the extended methods and changed signatures introduced in PR MapServer/MapServer#5078
This PR updates the documentation for colorObj in SWIG- and PHP- Mapscripts to reflect the alpha component, including the extended methods and changed signatures introduced in PR MapServer/MapServer#5078
Adds tests related to Mapserver PR MapServer/MapServer#5078
ejn
force-pushed
the
mapscriptColorAlpha70
branch
from
e2b2eb4
to
d7c26bc
Compare
Changes in the SWIG bindings: - add the methods setRGBA() and toHexWithAlpha() - extend the method setHex() to accept #rrggbbaa hex values Changes in the PHP bindings: - add the methods setRGBA() and toHexWithAlpha() - adds the methods toHex() and setHex() which were already in the SWIG bindings
Instead of introducing separate setRGBA and toHexWithAlpha we alter the existing method setRGB to take an optional fourth argument and toHex such that it outputs a 4-byte hex string if the alpha value is other than 255 and a 3-byte hex string as previously if the alpha is the default 255.
|
@tbonfort I rebased this PR to the current branch-7-0 to kick Travis into life. Since the extended method signatures are documented (MapServer/MapServer-documentation#114 got merged before this PR) I guess this one now counts as a bugfix too? ;-O |
MapScript colorObj SWIG/PHP alignment and methods with alpha (#5078)
|
@tbonfort Thanks! Please also merge MapServer/msautotest_DEPRECATED#30 |
Extend PHP Mapscript tests for color alpha values (MapServer/MapServer#5078)
Changes in the SWIG bindings:
Changes in the PHP bindings:
Fix for #5077
Still TODO: