play-co / devkit-core

11 stars 17 forks source link

MultiplyFilter has no effect on some views #20

Open lunarraid opened 9 years ago

lunarraid commented 9 years ago
import ui.View as View;
import ui.ImageView as ImageView;
import ui.ImageScaleView as ImageScaleView;
from ui.filter import MultiplyFilter;

exports = Class( GC.Application, function()
{
    this.initUI = function()
    {
        // This app has only been tested in the Chrome Browser on OSX.

        var colorFilter = new MultiplyFilter( { r: 255, g: 255, b: 0 } );

        // Regular ImageView, filter works as expected

        var image = new ImageView({
            superview: this,
            image: "resources/images/round-rectangle.png",
            x: 0,
            y: 0,
            width: 128,
            height: 128
        });

        image.setFilter( colorFilter );

        // ImageScaleView with no slicing applied, filter works as expected

        var nonScaledImage = new ImageScaleView({
            superview: this,
            x: 128,
            y: 0,
            width: 128,
            height: 128,
            image: "resources/images/round-rectangle.png"
        });

        nonScaledImage.setFilter( colorFilter );

        // ImageScaleView with slicing, filter has no effect

        var scaledImage = new ImageScaleView({
            superview: this,
            x: 256,
            y: 0,
            width: 128,
            height: 128,
            image: "resources/images/round-rectangle.png",
            scaleMethod: "9slice",
            sourceSlices: {
                horizontal: { left: 30, center: 2, right: 30 },
                vertical: { top: 30, middle: 2, bottom: 30 }
            }
        });

        scaledImage.setFilter( colorFilter );

        // Regular View with a child inside. Filter has no effect, though I cannot tell
        // from the docs whether this is intended behavior or not. Including just in case.

        var container = new View({
            superview: this,
            x: 0,
            y: 128,
            width: 128,
            height: 128
        });

        container.setFilter( colorFilter );

        new ImageView({
            superview: container,
            image: "resources/images/round-rectangle.png",
            x: 0,
            y: 0,
            width: 128,
            height: 128
        });

    };
} );
lunarraid commented 9 years ago

I have done some digging into this, and I believe in the case of the ImageScaleView, the issue lies here:

https://github.com/gameclosure/timestep/blob/master/src/ui/ImageScaleView.js#L622

By calling drawImage on the context directly instead of calling the render() method on the source image, the code is completely bypassing the filter rendering that the Image class provides.

jwilm commented 9 years ago

We should probably just have ImageScaleView create a new texture and then be rendered the normal way. 8 extra draw calls for a 9 sliced image seems bad.

lunarraid commented 9 years ago

That is a thought. I also have a patch in the works to fix the draw calls. A cached texture would certainly be faster for static content, though I am curious what would acquire more overhead when, say, tweening the width and height of an ImageScaleView. Perhaps a flag for whether or not to cache?

jwilm commented 9 years ago

Is tweening slice sizes a common thing?

lunarraid commented 9 years ago

Slice sizes, I wouldn't imagine so, but the width and height of the dialog itself, yes, which would invalidate the cached texture and require a new texture instantiation. It's an interesting problem, though I would imagine with most ImageScaleViews remaining static, your solution would probably be best for most use cases.

jwilm commented 9 years ago

Why would scaling a derived texture versus a texture taken straight from raw image data be any different? Both textures would maintain their original (max?) size in memory. I'm not actually pushing for going the derived texture route right now. There are some issues on the native side that make it difficult to do properly at this point. Of course, a less performant JS-only solution would be possible with offscreen canvas and data URIs. The performance hit would be upfront (base64 is expensive), but subsequent renders would be highly performant.

lunarraid commented 9 years ago

Ah, shoot, I only just realized what you meant. I thought we were talking about caching the entire scaled image as a texture, not caching the individual filtered components. Yes, caching the filtered components would definitely be a huge win.

jwilm commented 9 years ago

I'm not so worried about the filtered components for rendering a plain ImageView. The ImageScaleView has a lot more going on that you may or may not be aware of. This is best illustrated by the scaleMethod property which can have the following values: none, stretch, cover, contain, tile, 9slice, 6slice, 3slice, and 2slice. 9 slicing it the most computationally intensive and that is the primary case I'm thinking of here. If the layout of an image was done once and a texture cached for it (in this case, all 9 components on one texture), a plain ImageView could be used efficiently and with filtering. The same idea holds for the other scale methods, although the gains may not be as dramatic (or they may not be gains at all!).