(In reply to akash hiremath from comment #0)
> Created attachment 303204 [details] [review] [review]
> smooth palette operation for gegl. (Not complete.)
> 
> smooth palette operation for gegl.
> 
> This patch contains a port of smooth-palette from GIMP to GEGL.
> 
> presently the palette overlap on the whole image with specified size. I need
> help is setting output pad size.

About the patch:

- the first step of this filter is to randomly fetch pixels on the entire input buffer, so using a GEGL_OP_AREA_FILTER as base class is useless, use a GEGL_OP_FILTER instead.

- since pixels are fetched only inside the input buffer, using GEGL_ABYSS_CLAMP as abyss policy for the sampler is useless, use GEGL_ABYSS_NONE instead.

- why allocating memory to store the entire input buffer ? you only need the pixels randomly fetched to create the palette. You can use gegl_buffer_set (or even gegl_buffer_set_pattern) to fill the output buffer with the sorted palette.

- the output buffer size is determined by the rectangle returned by the get_bounding_box function ; your implementation is wrong (it returns the size of the input, which is not what you want)

(In reply to Thomas Manni from comment #1)
> (In reply to akash hiremath from comment #0)
> > Created attachment 303204 [details] [review] [review] [review]
> > smooth palette operation for gegl. (Not complete.)
> > 
> > smooth palette operation for gegl.
> > 
> > This patch contains a port of smooth-palette from GIMP to GEGL.
> > 
> > presently the palette overlap on the whole image with specified size. I need
> > help is setting output pad size.
> 
> About the patch:
> 
> - the first step of this filter is to randomly fetch pixels on the entire
> input buffer, so using a GEGL_OP_AREA_FILTER as base class is useless, use a
> GEGL_OP_FILTER instead.
> 
> - since pixels are fetched only inside the input buffer, using
> GEGL_ABYSS_CLAMP as abyss policy for the sampler is useless, use
> GEGL_ABYSS_NONE instead.
> 
> - why allocating memory to store the entire input buffer ? you only need the
> pixels randomly fetched to create the palette. You can use gegl_buffer_set
> (or even gegl_buffer_set_pattern) to fill the output buffer with the sorted
> palette.
> 
> - the output buffer size is determined by the rectangle returned by the
> get_bounding_box function ; your implementation is wrong (it returns the
> size of the input, which is not what you want)

I had problems in understanding gegl internals previously. Hence the poor code. 
Your replay cleared many things. I thought get_bounding_box returns original image size.
Thanks for the code review. I'll fix them as soon as i finish my exams.

Created attachment 318862 [details] [review]
Updated plug-in.

Updated plug-in. But I still don't know how to set output size. 
I tried setting using 'get_bounding_box' but it overwrite on the original image. Is it possible to create entirely new image? like, it should open new tab in gimp. I know gegl is not specifically only for Gimp, so 'opening tab' is not gegl part. need help regarding output image size.

Review of attachment 318862 [details] [review]:

Hi Akash,

We have had a port of the smooth palette plugin to GEGL today, without creating an operation (bug 774807). But I believe your operation can still be of use and could replace the implementation on the GIMP plugin.

Also you should not bother about the output buffer size from inside the operation. I imagine that you are testing with the "GEGL tool" in GIMP, and this tool is made for filter-like operation (overwriting the original image). A palette operation is not this type of operation and would therefore be simply not listed in the GEGL tool in the end. It would be directly used in other pieces of code.

As such, in your operation, I think you can get rid of the "height" property. Just make the height of the output buffer always 1. This will be up to the using code to do whatever it wants with the result (like duplicating the result into many "lines" it to make a usable image).

Also rename the "width" parameter into "n_colors" for instance. Basically conceptually, we should not consider the output as an image with a width and a height, but as an array of colors (and it turns out this array is stored as a GeglBuffer).

I also don't think that the depth ("ntries" parameter) is a good one. First because the current reordering is crap anyway, and really it is not be that long to reorder so few elements so really limiting with a number of tries seems like useless. Finally I'm not even sure if we should bother about sorting at the operation level. Maybe more interesting would be to not have duplicate colors (which the current algorithm does not care about at all currently).
But maybe we can leave this ntries parameter for now because Thomas Manni wanted to propose an alternative version for the reordering (which we can later move into the GEGL operation). Let's see what he comes up with first.

By the way, "palette" has a single 'l', not 2. ;-)

Well hold on to the patch. Some were noting that GIMP has the "Import Palette" function which can make a real palette from an image. Checking up the code for this piece of code, it is actually a lot better than whatever the current smooth-palette plugin is doing.

It actually check every pixel, order them by the most used first and return the XXX most used colors (where XXX is the requested number, basically the equivalent of "width" in your operation). There is not this whole randomized sampling crap from the current plugin (which makes that every palette creation is different and non-predictable).

I think that if we wanted to make a palette creation GEGL operation out of GIMP code, this is the code which should be used, not the one in the smooth-palette plugin.
You can find the code in gimp_palette_import_from_image() from app/core/gimppalette-import.c
This is the code which should be turned into a GEGL operation (it is already GEGLified code) if we want this IMO.

But let's wait for others to give an opinion before hacking further.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gegl/issues/27.