improving colorize-alpha #1371

Closed
springmeyer opened this Issue Aug 3, 2012 · 24 comments

Projects

None yet

6 participants

Owner

In addition to #1369, we should focus on improving the behavior of colorize-alpha.

Problems include:

  1. Fully alpha source is impacting background layers when it should not.
  2. Partially alpha source is recalculating source rgba based on background alpha rather than blending with it afterword
Owner

previously looked like:

After b43697f now looks like:

@springmeyer springmeyer pushed a commit that referenced this issue Aug 3, 2012
Dane Springmeyer colorize-alpha: only recolor if source alpha is > 0 to avoid overpain…
…ted background - refs #1371
b43697f
Owner

/cc @tmcw, @rsudekum, @hrwgc, @ian29 as I know they may be using this feature and have hit this.

Member
bsudekum commented Aug 3, 2012

Nice, good update @springmeyer. Any plans to implement control over the generated colors?

Contributor
tmcw commented Aug 3, 2012

@rsudekum possibly; I'm hoping that this can be implemented as a separate color-spin operation - it's tricky to have agg-level comp-ops that take parameters too afaik; @springmeyer has more C++ foo here.

Owner

Problem #2 is hard to solve because the method will radically differ depending on feature level compositing or style level.

e.g.: style level comp-op vs something like marker-comp-op.

I'm tending to think that only style level will be feasible. I can achieve this improvement with style level:

Owner

@tmcw - right, this is looking more like a problem to be solved by image-filters because image filters work on the whole buffer and when created will trigger a new internal, fully alpha, buffer to be created. And because, as you know, they can accept options as each one is quite custom. Unless you want heatmap like effects between different layers, I think this may be a better route.

Owner

@tmcw, yeah, yeah, this should be moved to an image-filter with a proper parser to accept args for the ramp.

So, I think I'm going to stop hacking at the blend_pix function, but for anyone curious I got the above image with a blend_pix like:

{
    if(cover < 255)
    {
        sr = (sr * cover + 255) >> 8;
        sg = (sg * cover + 255) >> 8;
        sb = (sb * cover + 255) >> 8;
        sa = (sa * cover + 255) >> 8;
    }
    if (sa > 0)
    {
        calc_type da = p[Order::A];
        calc_type s1a = base_mask - sa;

        p[Order::R] = (value_type)(sr + ((p[Order::R] * s1a + base_mask) >> base_shift));
        p[Order::G] = (value_type)(sg + ((p[Order::G] * s1a + base_mask) >> base_shift));
        p[Order::B] = (value_type)(sb + ((p[Order::B] * s1a + base_mask) >> base_shift));
        p[Order::A] = (value_type)(sa + da - ((sa * da + base_mask) >> base_shift));

        // http://en.wikipedia.org/wiki/File:HSV-RGB-comparison.svg
        if (sa < 64) {
            p[Order::G] = ((sa - 64) * 4);
            p[Order::B] = 255;
        }
        if (sa >= 64 && sa < 128) {
            p[Order::G] = 255;
            p[Order::B] = 255 - ((sa - 64) * 4);
        }
        if (sa >= 128 && sa < 192) {
            p[Order::R] = ((sa - 128) * 4);
            p[Order::G] = 255;
        }
        if (sa >= 192) {
            p[Order::R] = 255;
            p[Order::G] = 255 - ((sa - 192) * 4);
        }
    }
}
Owner

@artemp - can I pass this to you next to comment on (moving to image-filters asap)?

@tmcw - any ideas on an interface in carto to passing colors?

@tmcw - overall my feeling is this functionality, while cool, might not be ready for a TileMill release - especially if it needs to be moved to a different api - would you be amenable to removing from mapnik-reference for now until things settle out?

@artemp artemp was assigned Aug 14, 2012
Owner

bump @tmcw and @artemp

Owner

I'm going to disable colorize-alpha for now until this is more clear - to ensure the least amount of breakages given imminent mapnik and tilemill releases. Will confirm that a move to image-filters make sense after 2.1 release and the will get it done then.

@springmeyer springmeyer pushed a commit that referenced this issue Aug 15, 2012
Dane Springmeyer disable colorize-alpha comp-op as per #1371 4cf1484
@springmeyer springmeyer referenced this issue in tilemill-project/tilemill Sep 21, 2012
Open

Colorize-alpha #1715

Owner
artemp commented Mar 15, 2013

@springmeyer - not sure this can work as an image-filter - we need to apply "colorize" for each individual set-pixel call.

Owner
artemp commented Mar 15, 2013

@hrwgc - cool map!

Owner
artemp commented Mar 20, 2013

@springmeyer @tmcw @hrwgc

colorize-alpha image filter

comp-op="src-over"
colorise-alpha-src-over

comp-op="multiply"
colorize-alpha-multiply

We're currently expecting two args (which are ignored)

image-filters="colorize-alpha(red,blue)" 

We probably should support more than 2 color stops? Also original 'rainbow' gradient looks good...
@tmcw - do you have some ideas for syntax/options here ?

Owner
artemp commented Mar 20, 2013

542805e

 <Style name="style" comp-op="src-over" image-filters="colorize-alpha(blue,cyan,lightgreen, yellow, orange, red)">

colorize

Owner
artemp commented Mar 21, 2013

fe092ac - implements optional 'offset'

<Style name="style" comp-op="src-over" image-filters="colorize-alpha(blue 30%, cyan, yellow 0.7 , rgb(0%,80%,0%) 90%)">
    <Rule>
      <PointSymbolizer  file="../images/marker.png" allow-overlap="true"/>
    </Rule>
  </Style>

color-stops

hrwgc commented Mar 21, 2013

This is incredible @artemp

Owner
artemp commented Apr 8, 2013

Hmm.. this looks like carto rather than Mapnik issue - the mentioned style
works fine for me. ??

On 5 April 2013 19:37, Tom MacWright notifications@github.com wrote:

I'm getting a parse error from Mapnik with any colorize-alpha list with
more than two stops: example: https://gist.github.com/5321572

https://a248.e.akamai.net/camo.github.com/0d9f8e374d8f01b886621007669c35a3e3a5e276/687474703a2f2f646c2e64726f70626f782e636f6d2f752f36383035392f53637265656e73686f74732f3173765f377235376f6a725f2e706e67


Reply to this email directly or view it on GitHubhttps://github.com/mapnik/mapnik/issues/1371#issuecomment-15973046
.

Owner

Comment was removed so I think he figured that out.

Owner

@artemp - what do you think about colorize alpha with no arguments being valid and defaulting to the original rainbow?

Owner
artemp commented Apr 9, 2013

@springmeyer - we should add 'rainbow' filter per original implementation - using colorize-alpha which is based on CSS linear gradient without arguments doesn't feel like clean design.

How about:

image-filters="rainbow()" 

?

Owner

Hmm, maybe we need better error messages then either in carto/mapnik or both.

For example, agg-stack-blur also needs args and carto catches this nicely:

Screen Shot 2013-04-09 at 9 13 37 AM

But not for colorize-alpha:

Screen Shot 2013-04-09 at 9 17 48 AM

Screen Shot 2013-04-09 at 9 19 20 AM

@springmeyer springmeyer pushed a commit that referenced this issue Sep 27, 2013
Dane Springmeyer add missing colorize-alpha changelog entry - refs #1371 43a52e1
@springmeyer springmeyer pushed a commit that referenced this issue Sep 27, 2013
Dane Springmeyer move colorize-alpha to visual test suite - refs #1371 2287b0a
Owner

tests added, bug fixed, single arg constructor added, closing.

@herm herm added a commit that referenced this issue Oct 13, 2013
@herm herm Merge commit 'ccd17a3bf32c03d6c3040fa8e8413c22c1a9d1c2' into hb-merge
* commit 'ccd17a3bf32c03d6c3040fa8e8413c22c1a9d1c2': (11 commits)
  add link to SVG gradient stops in colorize-alpha notes
  fix image-filter serialization for scale-hsla and colorize-alpha
  add missing colorize-alpha changelog entry - refs #1371
  remove debug prints
  optimize case where image_view is used but is not a subset - refs #2024
  optimize webp pixel copy by moving row chunks rather than pixels - refs #2024
  iwyu
  travis: run tests
  build 2.3.x branch on travis
  fix #2024 (TODO - avoid image copy for lossy encoding of views)
  ...
47cb954
@herm herm added a commit that referenced this issue Oct 13, 2013
@herm herm Merge commit '2287b0a7b9eaff2cbb880edcf67a002185013343' into hb-merge
* commit '2287b0a7b9eaff2cbb880edcf67a002185013343':
  move colorize-alpha to visual test suite - refs #1371

Conflicts:
	tests/visual_tests/test.py
afbd5a0
@herm herm added a commit that referenced this issue Oct 13, 2013
@herm herm Merge commit '423a8007ba5afe9e05820c0038a0fae98f1b36ca' into hb-merge
* commit '423a8007ba5afe9e05820c0038a0fae98f1b36ca':
  another colorize-alpha test - this one likely exposing a bug - refs #1371

Conflicts:
	tests/visual_tests/test.py
e764036

great

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