Conversation
|
jaladh-singhal
left a comment
There was a problem hiding this comment.
Looks great! I agree with Luisa's comment about DCE stuff.
I noticed a bug: the fill color suddenly becomes opaque (instead of continuously becoming transparent) when you keep zooming in and are between 30 and 26 degrees FOV. I identified the reason for it in the logic (see my comment below).
| if (!fov) return {style:Style.FILL, color:toRGBAString([r,g,b,1])}; | ||
| if (fov<26) return {style:Style.DESTINATION_OUTLINE, color:toRGBAString([r,g,b,1])}; | ||
|
|
||
| let alpha= .7; |
There was a problem hiding this comment.
if fov >= 26 and fov <=30, alpha becomes .7 which breaks the transparency increasing when zoomming in behavior. Perhaps this should be .18.
Or L461 can be if (fov<30) to prevent this 26-30 gap
| if (fov<26) return {style:Style.DESTINATION_OUTLINE, color:toRGBAString([r,g,b,1])}; | ||
|
|
||
| let alpha= .7; | ||
| if (fov>170) alpha= .7; | ||
| else if (fov>150) alpha= .68; | ||
| else if (fov>130) alpha= .65; | ||
| else if (fov>110) alpha= .58; | ||
| else if (fov>90) alpha= .55; | ||
| else if (fov>70) alpha= .45; | ||
| else if (fov>50) alpha= .35; | ||
| else if (fov>40) alpha= .25; | ||
| else if (fov>30) alpha= .20; | ||
| return {style:Style.FILL, color: toRGBAString([r,g,b,alpha])}; | ||
| } |
There was a problem hiding this comment.
Actually...I was wondering if there's some formula to map fov to our alpha range (0.2-0.7) and asked ChatGPT:
// Linearly interpolate alpha between these breakpoints:
const MIN_FOV = 26;
const MAX_FOV = 170;
const MIN_ALPHA = 0.2;
const MAX_ALPHA = 0.7;
// Tiny FOV → outline
if (fov < MIN_FOV) {
return {
style: Style.DESTINATION_OUTLINE,
color: toRGBAString([...baseRGBA, 1])
};
}
// Fill with interpolated alpha
const t = Math.min(Math.max((fov - MIN_FOV) / (MAX_FOV - MIN_FOV), 0), 1); // normalized fov in [0,1]
const alpha = MIN_ALPHA + t * (MAX_ALPHA - MIN_ALPHA);
return {
style: Style.FILL,
color: toRGBAString([...baseRGBA, alpha])
};And then it's easier to play with the alpha values. I think MIN_ALPHA = 0.1 will actually be better to show transparently filled to outline transition.
There was a problem hiding this comment.
I considered do something like this but it got forgotten after the proof-of-concept started working. I will try it.
jaladh-singhal
left a comment
There was a problem hiding this comment.
Thanks for the update. looks good and the bug is gone.
| const [r,g,b]= toRGBA(color); | ||
| const fov= getFoV(plot); | ||
| if (!fov) return {style:Style.FILL, color:toRGBAString([r,g,b,1])}; | ||
| if (autoUsesOnlyOutline || fov<30) return {style:Style.DESTINATION_OUTLINE, color:toRGBAString([r,g,b,1])}; |
There was a problem hiding this comment.
This number should come from the constant so that it doesn't break if someone changes that in getAlpha (you probably need to move the constants out of the functions):
| if (autoUsesOnlyOutline || fov<30) return {style:Style.DESTINATION_OUTLINE, color:toRGBAString([r,g,b,1])}; | |
| if (autoUsesOnlyOutline || fov<MIN_FOV) return {style:Style.DESTINATION_OUTLINE, color:toRGBAString([r,g,b,1])}; |
There was a problem hiding this comment.
@robyww Before you merge: I noticed your last commit changed minFov in getAlpha to 25, but not in L466, they must be the same. That's why I suggested defining them as MIN_FOV constant and using it at these both places to prevent getting them out of sync.
minFov = 25;
if (autoUsesOnlyOutline || fov<minFov) return {style:Style.DESTINATION_OUTLINE, color:toRGBAString([r,g,b,1])};
const alpha= getAlpha(fov, {minFov});There was a problem hiding this comment.
They don't have to be the same. if FOV < 30 then the getAlpha() never gets called. By making minFov less then 30 then it approaches the "fill disappear effect" that you suggested. However, it will never actually gets there, it switches over first. We don't want to get to the "fill disappear" because there will be a FOV that seems buggy.
There was a problem hiding this comment.
We could accomplish a similar effect by making minFov 30 and minAlpha 5. I might plan with it more at another time.
| const MIN_FOV = 30; | ||
| const MAX_FOV = 170; | ||
| const MIN_ALPHA = 0.2; | ||
| const MAX_ALPHA = 0.7; |
There was a problem hiding this comment.
something worth trying, I have a hunch this will give the cleanest transition from transparently filled to only outlined:
| const MIN_FOV = 30; | |
| const MAX_FOV = 170; | |
| const MIN_ALPHA = 0.2; | |
| const MAX_ALPHA = 0.7; | |
| const MIN_FOV = 25; | |
| const MAX_FOV = 170; | |
| const MIN_ALPHA = 0.01; | |
| const MAX_ALPHA = 0.7; |
|
wow, this is SUPER COOL. works GREAT! tried all three and it is nice!! |
37bf4f1 to
9a4d0a1
Compare
- Auto will show MOC filled with a transparency based on FOV - Auto will show MOC as outline when the user zooms in - includes response to feedback - includes some peliminary DCE updates clean up and bug fixing
9a4d0a1 to
65f1d49
Compare

Firefly-1806: add MOC auto display mode
Testing