RemoveColormap API function added

[ToppDev]

Following up on #341 I implemented the feature.

There are 2 issues I would like your input with:

1. Where or how would you define the number of built-in colormaps? I just defined it as IMPLOT_CMAP_COUNT_BUILT_IN directly above the Initialize function. However there might be a better place to put it, to fit into the library structure.
2. I actually don't remove the respective entry out of the tables, but rather call the RebuildTables function at the end. That was kinda out of laziness from my side. If you prefer it, I can still implement a void _RemoveTable(ImPlotColormap cmap) function which only removes the needed entries.

[ToppDev]

any chance to get this into the master branch? (then I could put my submodules on the github master again instead of keeping my own repo)

[epezent]

Hey @ToppDev -- so sorry I've let this go unattended for this long. I'm ready to get this merged. There's one issue, however. As it is currently implemented, the current ImPlotStyle::Colormap index or any user cached ImPlotColormap indices (i.e. return values of AddColormap) are invalidated if a colormap with a lower index is removed. You can see this in the GIF below, where the current ImPlotStyle::Colormap index refers to RGB initially but then CMYK once Dracula is removed.

I have two possible solutions:

1) ImPlotColormaps become hashed IDs instead of indices. We would need to change around the internals of ImPlotColormapData to use ImPool or some other hash based storage container where we currently use ImVector. We'd also need to figure out how to handle the built-in enums which are currently hard coded indices. And we'd need a new unsigned sentinal value for specificying the "current" colormap in our public API (we currently use IMPLOT_AUTO which is -1).

2) We keep the ImVector storage containers but instead of erasing their elements when a colormap is removed, we mark that index as "available". When a new colormap is added, we search for the next available index. If one exists, we reuse it, otherwise we added new elements to the vectors. This may make the Count variable less useful, and we'd need to check if an index is used where we iterate the colormap vectors (e.g. inside of the colormap style editor and metrics windows).

I think I am leaning toward (2), but I'm curious to hear your thoughts.

[ToppDev]

Hey, sorry for also coming back late at you. Thanks for bringing this issue to my attention, I somehow missed it.

I also think your second solution sounds better. While the first solution is probably cleaner, it would affect a lot of code. It also would break the compatibility to e.g. IMPLOT_AUTO as you mentioned and even if we choose a different value, it would be not as intuitive for the user I guess.

I will try implementing the second solution till next week and see how much code is affected by it

[ToppDev]

so I looked into the problem some more. I think I underestimated the drawback of the Count variable being less useful as you mentioned earlier. It basically introduces all kind of inconsistencies which have to be accounted for and this bloats up the code.

Therefore I actually wondered if it is the task of the library to ensure, that if the user decided to store a colormap index, that the index keeps the same. I made a commit so that the Style Editor correctly decrements the gp.Style.Colormap variable. However I think it is the users responsibility to decrement his variables. Wrong usage of the API is anyway protected by all the IM_ASSERT_USER_ERROR checks.

What do you think about this?

[epezent]

However I think it is the users responsibility to decrement his variables.

I think this would be a bad choice and would cause several headaches down the road. I am stilll in favor of (2) described above, or not supporting this particular feataure until we have a better alternative.

As I imagine the typical need for RemoveColormap is to replace an existing colormap with new colors, what if we instead provided a mechanism to "update" a colormap or replace colors in an already existing colormap?

[epezent]

Somewhat to this, I recently had the idea that it would be neat if a reversed colormap could be specificied by negating the value of a Colormap index, e.g. PushColormap(-ImPlotColormap_Viridis). Upon receving a neative index, we would still use the positive index to for array indexing purposes, but we'd set a flag to instruct ImPlot to interpolate the map in reverse. This would require that all colormap indices be greater than 0, so we'd use 0 as our "current" sentinal value isntead of IMPLOT_AUTO=-1.

[epezent]

Somewhat to this, I recently had the idea that it would be neat if a reversed colormap could be specificied by negating the value of a Colormap index, e.g. PushColormap(-ImPlotColormap_Viridis). Upon receving a neative index, we would still use the positive index to for array indexing purposes, but we'd set a flag to instruct ImPlot to interpolate the map in reverse. This would require that all colormap indices be greater than 0, so we'd use 0 as our "current" sentinal value isntead of IMPLOT_AUTO=-1.

[ToppDev]

Sorry to also leave this unattended for so long.

I do not really have time to continue this, as it become a bigger problem than I anticipated at the start. So if anyone wants to take over my work, feel free. Otherwise I would just close this PR.

Thanks for your guidance though @epezent