Fix #33344. Keep Hue value even when Saturation is 0, otherwise converting color between RGBA/HSLA/HSVA will lead to the loss of Hue.

Since we bring back Hue value properly, 23bd5fee0f needs to be reverted, our old tests are good.
This commit is contained in:
rebornix
2017-08-30 15:43:30 -07:00
parent 3416d0d70c
commit ebd61ccd4f
3 changed files with 37 additions and 9 deletions

View File

@@ -260,8 +260,22 @@ export class Color {
}
readonly rgba: RGBA;
get hsla(): HSLA { return HSLA.fromRGBA(this.rgba); }
get hsva(): HSVA { return HSVA.fromRGBA(this.rgba); }
private _hsla: HSLA;
get hsla(): HSLA {
if (this._hsla) {
return this._hsla;
} else {
return HSLA.fromRGBA(this.rgba);
}
}
private _hsva: HSVA;
get hsva(): HSVA {
if (this._hsva) {
return this._hsva;
}
return HSVA.fromRGBA(this.rgba);
}
constructor(arg: RGBA | HSLA | HSVA) {
if (!arg) {
@@ -269,8 +283,10 @@ export class Color {
} else if (arg instanceof RGBA) {
this.rgba = arg;
} else if (arg instanceof HSLA) {
this._hsla = arg;
this.rgba = HSLA.toRGBA(arg);
} else if (arg instanceof HSVA) {
this._hsva = arg;
this.rgba = HSVA.toRGBA(arg);
} else {
throw new Error('Invalid color ctor argument');
@@ -278,7 +294,7 @@ export class Color {
}
equals(other: Color): boolean {
return !!other && RGBA.equals(this.rgba, other.rgba);
return !!other && RGBA.equals(this.rgba, other.rgba) && HSLA.equals(this.hsla, other.hsla) && HSVA.equals(this.hsva, other.hsva);
}
/**

View File

@@ -23,9 +23,9 @@ suite('Color', () => {
let color1 = new Color(new HSLA(60, 1, 0.5, 1)), color2 = new Color(new HSLA(0, 0, 0.753, 1));
assert.deepEqual(color1.hsla, Color.getLighterColor(color1, color2).hsla);
assert.deepEqual(new HSLA(0, 0, 0.918, 1), Color.getLighterColor(color2, color1).hsla);
assert.deepEqual(new HSLA(0, 0, 0.916, 1), Color.getLighterColor(color2, color1).hsla);
assert.deepEqual(new HSLA(0, 0, 0.851, 1), Color.getLighterColor(color2, color1, 0.3).hsla);
assert.deepEqual(new HSLA(0, 0, 0.980, 1), Color.getLighterColor(color2, color1, 0.7).hsla);
assert.deepEqual(new HSLA(0, 0, 0.981, 1), Color.getLighterColor(color2, color1, 0.7).hsla);
assert.deepEqual(new HSLA(0, 0, 1, 1), Color.getLighterColor(color2, color1, 1).hsla);
});
@@ -47,7 +47,7 @@ suite('Color', () => {
assert.deepEqual(new HSLA(60, 1, 0.284, 1), Color.getDarkerColor(color1, color2, 1).hsla);
// Abyss theme
assert.deepEqual(new HSLA(355, 0.875, 0.157, 1), Color.getDarkerColor(Color.fromHex('#770811'), Color.fromHex('#000c18'), 0.4).hsla);
assert.deepEqual(new HSLA(355, 0.874, 0.157, 1), Color.getDarkerColor(Color.fromHex('#770811'), Color.fromHex('#000c18'), 0.4).hsla);
});
test('luminance', function () {
@@ -179,6 +179,12 @@ suite('Color', () => {
assert.deepEqual(HSVA.fromRGBA(new RGBA(0, 128, 128, 1)), new HSVA(180, 1, 0.502, 1));
assert.deepEqual(HSVA.fromRGBA(new RGBA(0, 0, 128, 1)), new HSVA(240, 1, 0.502, 1));
});
test('Keep hue value when saturation is 0', () => {
assert.deepEqual(HSVA.toRGBA(new HSVA(10, 0, 0, 0)), HSVA.toRGBA(new HSVA(20, 0, 0, 0)));
assert.deepEqual(new Color(new HSVA(10, 0, 0, 0)).rgba, new Color(new HSVA(20, 0, 0, 0)).rgba);
assert.notDeepEqual(new Color(new HSVA(10, 0, 0, 0)).hsva, new Color(new HSVA(20, 0, 0, 0)).hsva);
});
});
suite('Format', () => {

View File

@@ -129,6 +129,7 @@ class SaturationBox extends Disposable {
private width: number;
private height: number;
private monitor: GlobalMouseMoveMonitor<IStandardMouseMoveEventData>;
private _onDidChange = new Emitter<{ s: number, v: number }>();
readonly onDidChange: Event<{ s: number, v: number }> = this._onDidChange.event;
@@ -154,22 +155,24 @@ class SaturationBox extends Disposable {
this._register(dom.addDisposableListener(this.domNode, dom.EventType.MOUSE_DOWN, e => this.onMouseDown(e)));
this._register(this.model.onDidChangeColor(this.onDidChangeColor, this));
this.monitor = null;
}
private onMouseDown(e: MouseEvent): void {
const monitor = this._register(new GlobalMouseMoveMonitor<IStandardMouseMoveEventData>());
this.monitor = this._register(new GlobalMouseMoveMonitor<IStandardMouseMoveEventData>());
const origin = dom.getDomNodePagePosition(this.domNode);
if (e.target !== this.selection) {
this.onDidChangePosition(e.offsetX, e.offsetY);
}
monitor.startMonitoring(standardMouseMoveMerger, event => this.onDidChangePosition(event.posx - origin.left, event.posy - origin.top), () => null);
this.monitor.startMonitoring(standardMouseMoveMerger, event => this.onDidChangePosition(event.posx - origin.left, event.posy - origin.top), () => null);
const mouseUpListener = dom.addDisposableListener(document, dom.EventType.MOUSE_UP, () => {
this._onColorFlushed.fire();
mouseUpListener.dispose();
monitor.stopMonitoring(true);
this.monitor.stopMonitoring(true);
this.monitor = null;
}, true);
}
@@ -221,6 +224,9 @@ class SaturationBox extends Disposable {
}
private onDidChangeColor(): void {
if (this.monitor && this.monitor.isMonitoring()) {
return;
}
this.paint();
}
}