set `active_pixel_bitmap` in a better way
jeff-regier opened this issue · 4 comments
As background, here's an illustration of how active pixels are set currently:
In the images below, the circles indicate the SkyPatch radius for each object. (They also serve to show where the objects are). The pixel outlines show the active pixels for each object.
With no minimum radius (Config(0.0))
With minimum radius of 8 pixels (default; Config())
Take-aways
The active pixel selection algorithm based on pixel significance (the part other than the minimum radius requirement) only selects a few of the brightest pixels in some objects and none at all in others. The minimum radius requirement is basically the only active pixel selection, since it usually contains a superset of the pixels selected based on significance. Also, sometimes the active pixels extend outside the patch radius (the patch radius is smaller than 8 pixels).
We should probably just use a heuristic similar to the one used for patch radius, but perhaps also taking extent of significant pixels into account.
Sounds good. Minor point: Since the sky patch is a square, we're using the term "radius" kind of loosely for it. Here "radius" denotes the radius of an L_\infty ball, i.e. a square with edge length 2 * radius.
Update: I finally had an implementation I was happy with a week and a half ago, but have since been working through a test failure that I've now finally gotten to the bottom of. I should have a PR ready in the next couple days.
Great! I'm looking forward to the PR.