RFC : Merge Filters about imaging HOT 3 CLOSED

About merging filters - I think it's really an interesting idea, I thought of something like this, but for now decided to keep things more simple.

Talking about performance I don't see a big advantage:

• toNRGBA does nothing if the image is already NRGBA, which is true for sequential functions call.
• Sharpen uses Blur, but in your example with another sigma, so recalculation needed anyway.
• image.NewNRGBA - not always needed, but things like gaussian blur cannot be done in-place. I don't think its a big performance penalty.
• different filters need different algorithms and don't fit one conception

But some things indeed could be optimized this way, for example do contrast and brightness adjustments at once.

img.Rotate(90) // Rotate Image 90 degree
rotation here changes image dimensions - this means a new pixel array, a new stride. in this case why not a function returning a new image?

Using functions instead of methods have a big advantage - you can pass any image that implements image.Image interface to imaging.Rotate. in your example img is a special type of image that has Rotate method.

from imaging.

About merging filters - I think it's really an interesting idea, I thought of something like this, but for now decided to keep things more simple.

I appreciate the simplicity but having to call imaging.Method(srcImage,args)` looked like duplication to me because am force to initialize a variable all the time

    firstStep := imaging.MethodA(srcImage, arg)
    secoundStep := imaging.MethodB(firstStep , arg)
    thirdStep := imaging.MethodC(secoundStep , arg)

Talking about performance I don't see a big advantage:

Valid point , don't like micro optimization.. should have done performance test before bring this up.

rotation here changes image dimensions - this means a new pixel array, a new stride. in this case why not a function returning a new image?

LGTM ... seems like a reasonable approach.

But some things indeed could be optimized this way, for example do contrast and brightness adjustments at once.

Do you want me to implement this ?

rotation here changes image dimensions - this means a new pixel array, a new stride. in this case why not a function returning a new image

That is the mail idea but it can be skipped. From my example it can as well be

    img, err := imaging.Open("src.png") // Open Image and Return  ImageSrc type

    filter, _ := imaging.Filter(img) // Start Filter
    imgGray, _ := filter.Grayscale()
    imgBlur, _ := filter.Blur(4.5)
    imgSharpen, _ := filter.Sharpen(3.0)
    imgBrigtness, _ := filter.Brightness(+5)
    imgContrast, _ := filter.Contrast(+2)

    imgRotate, _ := img.Rotate(90)                  // Rotate Image 90 degree
    imgBackground, _ := img.PasteCenter(backgroundImage) // Paste Image a new Background

This kind of flexibility can be useful because i scan simply call imgGray.Save("dst_gray.jpg") or imgBlur.Color(0xFFCCDD) etc which is different for the original image

Using functions instead of methods have a big advantage - you can pass any image that implements image.Image interface to imaging.Rotate. in your example img is a special type of image that has Rotate method.

This changes does not break the current API but rather than implement any code the functions would just be syntactic sugars

Finally

The current implementation & API is fantastic ... am just looking at ways we can make it better :)

from imaging.

Thanks for your feedback! :)

You can reuse a variable:

    img = imaging.MethodA(img, arg)
    img = imaging.MethodB(img, arg)
    img = imaging.MethodC(img, arg)

Do you want me to implement this ?

No, just counting pros and cons
For now i don't see a big advantage to change/update the API this way.

from imaging.