Hammarberg / pixcen

Low level Windows pixel editor for C64
http://binarybone.com/pixcen
GNU General Public License v3.0
53 stars 7 forks source link

Background Colour Selection is "wrong" #31

Open oziphantom opened 7 years ago

oziphantom commented 7 years ago

Hi I've loaded in a mostly light grey(0xF) image, the loading screen of Kung-Fu Master. The program always selects Black as the background colour. Looking at the code `int C64Interface::ImportHelper::CountTopColorsPerCell(BYTE col, int ceil) { int colcount[16], count=0; int colcount2[16]; ZeroMemory(colcount2, 16sizeof(int));

int cx,cy;

for(cy=0;cy<ysize/ycs;cy++)
{
    for(cx=0;cx<xsize/xcs;cx++)
    {
        if(CountColors(cx,cy,1,1,colcount)>=ceil)
        {
            for(int r=0;r<16;r++)
            {
                if(colcount[r])
                    colcount2[r]++;
            }
        }
    }
}

for(;;count++)
{
    int i = highindex(colcount2, 16);
    if(i == -1)break;

    colcount2[i] = -1;
    col[count] = BYTE(i);
}`

I think is the issue the >= in if(CountColors(cx,cy,1,1,colcount)>=ceil) means that in my case of MCol bitmap, only Cells that have 4 or more colours get counted into colcount2 and so Black wins, as it is in cells with the large text, which also have white and red. Thus black becomes the most common colour. However I would say Light Grey is about 6x as much as black. Changing the >= to a <= such that only cells with the max colour count and LESS are counted gives me a Light Grey background as expected.

Hammarberg commented 7 years ago

Hi and thanks for investigating!

As you noticed, the formula looks for the most common color in cells of 4 colors. From my earlier tests this have been the least destructive and best way of doing it so far. The most common color is not necessary the best background color on more complex images.

When exporting to and reimporting from PNG there should never be color destruction and that was the goal of the formula.

It is possible that both light grey and black are suitable for background in this case and the formula picks black. Unless you also spotted destruction going on? Then it could be a bug.

I have some ideas how to solve it. Pixcen is overdue for an import dialog with remap options. That would be a good place to pick/force a background. Or, a bit quicker without new GUI would be to test what colors can be non destructive background and then pick the most common in general of those.

oziphantom commented 7 years ago

Ah yeah ok, I see the horror case. I would just add a menu item under import of Force Background Colour.

I've been able to make a display mode that shows me what colours are used in the Image. I tried to get it on an accelerator but the Menu in the app won't update to match the res file... I've deleted it many times still the old one... Ah MFC.

Am I correct with 0 = background 1 = CRAM 2 = MCol 1 3 = MCol 2?

oziphantom commented 7 years ago

No its 0 = background 1 = MCol 1 2 = MCol 2 3 = CRAM

Hammarberg commented 7 years ago

Sorry for the late reply but I see you worked the mask order :-)

Conclusion for this issue I think is that remap and import color selection is needed. Remap is one of the more common requests.

oziphantom commented 7 years ago

Good news is I have a Remap system working for MC Bitmaps. I guess it might also be used on Hires Bitmap, but it would just be an invert function in that case. The code needs some clean up such that the menu items are only selectable when you are in MC Bitmap mode. It opens a menu that lists all the colours in the cells you have "marked" and gives you a list of Back, MCol1, MCol2 and CRAM.