From 14ca3587c90cd948518e1ef710d6b85d6289fcdb Mon Sep 17 00:00:00 2001
From: Kanit Wongsuphasawat <kanit.w@databricks.com>
Date: Wed, 11 Oct 2023 18:03:40 -0700
Subject: [PATCH] fix: support x/yOffset without x/y

---
 src/compile/mark/encode/position-rect.ts | 13 +++++-
 src/compile/scale/parse.ts               | 14 +------
 src/compile/scale/range.ts               | 50 +++++++++++++++++-------
 src/stack.ts                             | 16 ++++----
 4 files changed, 56 insertions(+), 37 deletions(-)

diff --git a/src/compile/mark/encode/position-rect.ts b/src/compile/mark/encode/position-rect.ts
index afbd417d48d..b2229b1433b 100644
--- a/src/compile/mark/encode/position-rect.ts
+++ b/src/compile/mark/encode/position-rect.ts
@@ -166,7 +166,14 @@ function positionAndSize(
   const hasSizeFromMarkOrEncoding = !!sizeMixins;
 
   // Otherwise, apply default value
-  const bandSize = getBandSize({channel, fieldDef, markDef, config, scaleType: scale?.get('type'), useVlSizeChannel});
+  const bandSize = getBandSize({
+    channel,
+    fieldDef,
+    markDef,
+    config,
+    scaleType: (scale || offsetScale)?.get('type'),
+    useVlSizeChannel
+  });
 
   sizeMixins = sizeMixins || {
     [vgSizeChannel]: defaultSizeRef(
@@ -190,7 +197,9 @@ function positionAndSize(
    */
 
   const defaultBandAlign =
-    scale?.get('type') === 'band' && isRelativeBandSize(bandSize) && !hasSizeFromMarkOrEncoding ? 'top' : 'middle';
+    (scale || offsetScale)?.get('type') === 'band' && isRelativeBandSize(bandSize) && !hasSizeFromMarkOrEncoding
+      ? 'top'
+      : 'middle';
 
   const vgChannel = vgAlignedPositionChannel(channel, markDef, config, defaultBandAlign);
   const center = vgChannel === 'xc' || vgChannel === 'yc';
diff --git a/src/compile/scale/parse.ts b/src/compile/scale/parse.ts
index a2334bcf7aa..e6baf799afa 100644
--- a/src/compile/scale/parse.ts
+++ b/src/compile/scale/parse.ts
@@ -1,7 +1,6 @@
-import {getMainChannelFromOffsetChannel, isXorYOffset, ScaleChannel, SCALE_CHANNELS, SHAPE} from '../../channel';
+import {ScaleChannel, SCALE_CHANNELS, SHAPE} from '../../channel';
 import {getFieldOrDatumDef, ScaleDatumDef, TypedFieldDef} from '../../channeldef';
 import {channelHasNestedOffsetScale} from '../../encoding';
-import * as log from '../../log';
 import {GEOSHAPE} from '../../mark';
 import {
   NON_TYPE_DOMAIN_RANGE_VEGA_SCALE_PROPERTIES,
@@ -56,17 +55,6 @@ function parseUnitScaleCore(model: UnitModel): ScaleComponentIndex {
     }
 
     let specifiedScale = fieldOrDatumDef && fieldOrDatumDef['scale'];
-    if (isXorYOffset(channel)) {
-      const mainChannel = getMainChannelFromOffsetChannel(channel);
-      if (!channelHasNestedOffsetScale(encoding, mainChannel)) {
-        // Don't generate scale when the offset encoding shouldn't yield a nested scale
-        if (specifiedScale) {
-          log.warn(log.message.offsetEncodingScaleIgnored(channel));
-        }
-        continue;
-      }
-    }
-
     if (fieldOrDatumDef && specifiedScale !== null && specifiedScale !== false) {
       specifiedScale ??= {};
       const hasNestedOffsetScale = channelHasNestedOffsetScale(encoding, channel);
diff --git a/src/compile/scale/range.ts b/src/compile/scale/range.ts
index a25d3824701..6b8b2b1267a 100644
--- a/src/compile/scale/range.ts
+++ b/src/compile/scale/range.ts
@@ -221,11 +221,39 @@ function parseScheme(scheme: Scheme | SignalRef): RangeScheme {
   return {scheme};
 }
 
+function fullWidthOrHeightRange(
+  channel: 'x' | 'y',
+  model: UnitModel,
+  scaleType: ScaleType,
+  {center}: {center?: boolean} = {}
+) {
+  // If step is null, use zero to width or height.
+  // Note that we use SignalRefWrapper to account for potential merges and renames.
+  const sizeType = getSizeChannel(channel);
+  const sizeSignal = model.getName(sizeType);
+  const getSignalName = model.getSignalName.bind(model);
+
+  if (channel === Y && hasContinuousDomain(scaleType)) {
+    // For y continuous scale, we have to start from the height as the bottom part has the max value.
+    return center
+      ? [
+          SignalRefWrapper.fromName(name => `0.5*${getSignalName(name)}`, sizeSignal),
+          SignalRefWrapper.fromName(name => `-0.5*${getSignalName(name)}`, sizeSignal)
+        ]
+      : [SignalRefWrapper.fromName(getSignalName, sizeSignal), 0];
+  } else {
+    return center
+      ? [
+          SignalRefWrapper.fromName(name => `-0.5*${getSignalName(name)}`, sizeSignal),
+          SignalRefWrapper.fromName(name => `0.5*${getSignalName(name)}`, sizeSignal)
+        ]
+      : [0, SignalRefWrapper.fromName(getSignalName, sizeSignal)];
+  }
+}
+
 function defaultRange(channel: ScaleChannel, model: UnitModel): VgRange {
   const {size, config, mark, encoding} = model;
 
-  const getSignalName = model.getSignalName.bind(model);
-
   const {type} = getFieldOrDatumDef(encoding[channel]) as ScaleFieldDef<string> | ScaleDatumDef;
 
   const mergedScaleCmpt = model.getScaleComponent(channel);
@@ -245,18 +273,7 @@ function defaultRange(channel: ScaleChannel, model: UnitModel): VgRange {
         }
       }
 
-      // If step is null, use zero to width or height.
-      // Note that we use SignalRefWrapper to account for potential merges and renames.
-
-      const sizeType = getSizeChannel(channel);
-      const sizeSignal = model.getName(sizeType);
-
-      if (channel === Y && hasContinuousDomain(scaleType)) {
-        // For y continuous scale, we have to start from the height as the bottom part has the max value.
-        return [SignalRefWrapper.fromName(getSignalName, sizeSignal), 0];
-      } else {
-        return [0, SignalRefWrapper.fromName(getSignalName, sizeSignal)];
-      }
+      return fullWidthOrHeightRange(channel, model, scaleType);
     }
 
     case XOFFSET:
@@ -374,6 +391,11 @@ function getOffsetStep(step: Step, offsetScaleType: ScaleType) {
 function getOffsetRange(channel: string, model: UnitModel, offsetScaleType: ScaleType): VgRange {
   const positionChannel = channel === XOFFSET ? 'x' : 'y';
   const positionScaleCmpt = model.getScaleComponent(positionChannel);
+
+  if (!positionScaleCmpt) {
+    return fullWidthOrHeightRange(positionChannel, model, offsetScaleType, {center: true});
+  }
+
   const positionScaleType = positionScaleCmpt.get('type');
   const positionScaleName = model.scaleName(positionChannel);
 
diff --git a/src/stack.ts b/src/stack.ts
index db05480795a..00e7bb3b38f 100644
--- a/src/stack.ts
+++ b/src/stack.ts
@@ -176,16 +176,16 @@ export function stack(m: Mark | MarkDef, encoding: Encoding<string>): StackPrope
       groupbyChannels.push(dimensionChannel);
       groupbyFields.add(dimensionField);
     }
+  }
 
-    const dimensionOffsetChannel = dimensionChannel === 'x' ? 'xOffset' : 'yOffset';
-    const dimensionOffsetDef = encoding[dimensionOffsetChannel];
-    const dimensionOffsetField = isFieldDef(dimensionOffsetDef) ? vgField(dimensionOffsetDef, {}) : undefined;
+  const dimensionOffsetChannel = dimensionChannel === 'x' ? 'xOffset' : 'yOffset';
+  const dimensionOffsetDef = encoding[dimensionOffsetChannel];
+  const dimensionOffsetField = isFieldDef(dimensionOffsetDef) ? vgField(dimensionOffsetDef, {}) : undefined;
 
-    if (dimensionOffsetField && dimensionOffsetField !== stackedField) {
-      // avoid grouping by the stacked field
-      groupbyChannels.push(dimensionOffsetChannel);
-      groupbyFields.add(dimensionOffsetField);
-    }
+  if (dimensionOffsetField && dimensionOffsetField !== stackedField) {
+    // avoid grouping by the stacked field
+    groupbyChannels.push(dimensionOffsetChannel);
+    groupbyFields.add(dimensionOffsetField);
   }
 
   // If the dimension has offset, don't stack anymore