Closed112

Babylon.jsにWebXR Depth Sensing Featureを実装し、コミットしていく作業ログ

にー兄さんにー兄さん

実は以前からこのissueは上がっていたのだが、先送りにされているなぁという風に見える
https://github.com/BabylonJS/Babylon.js/issues/11876

このissueがきっかけというわけではないんだけど、
前から気になっていたので使って見て、なんとなく実装方法を調べたらなんとなく行けそうな気がした
そんな流れでした

にー兄さんにー兄さん

とりあえず一通り自分の中では実装ができたので、PRのDraftを外してレビュー段階に突入
CIでlintが失敗していたので修正

にー兄さんにー兄さん

メンテナのRaananさんからコメントが届いた

Thank you so much for thie PR! Great work :-)
We try not to provide the dev directly with WebXR objects but abstract the usage as much as we can.
For example - CPU usage, it seems like we can "init" the XRCPUDepthInformation object internally, and then, once available, provide the getDepthInMeters directly, or at least abstract that to the dev.
Regarding GPU, we should not provide a WebGL2Texture to the user, but a babylon texture that the user can use. Internally, the user can have access to the internal texture, but the public API should be a babylon object and not a WebXR/WebGL object.
We should provide something extra to the dev, otherwise there is no difference between using the feature and implementing it yourself.

要約すると、

  • Babylon.jsではWebXR Device APIのメソッドをそのまま提供するのは避けている
  • 特別な機能を提供しないと実装する意味があまりなくなってしまう
  • だからCPU depth usageでは get depth in meterなどをダイレクトに提供したり、GPUはTextureにラップしたりしたい
にー兄さんにー兄さん

EngineにはWebGLTextureをInternalTextureに、InternalTextureはTextureに変換できる

ここら辺をWebGL系には実装したい

にー兄さんにー兄さん

ひとまずAPI仕様を提案してみた
前回と違い、イベント駆動でpush型での取得ではなく
pull型で取得するような感じ
img
どうかな。。。

にー兄さんにー兄さん

いろいろあって一回PRがクローズされた
ちょっと時間を空けすぎてしまったのと、おそらくbabylonのリポジトリでちょっとしたコミット整理が行われた?のかもしれない
PRのログもなんか変な感じになってて、GitHubのsync機能を使っても自分のforkリポジトリでコンフリクトが起きてしまっていた

にー兄さんにー兄さん

これでは本体にコミットできないのでローカルでコンフリクトを解決してupstreamとforkのmasterを同期させることに
記事はこちらを参考にしている
https://dev-yakuza.posstree.com/git/sync-fork-repository/

にー兄さんにー兄さん

githubのsyncを何回か使っていたからだろうか、すでにローカルにはupstreamURLが指定されていた
それをまずfetchした

にー兄さんにー兄さん

そのあと記事にあるようにローカルのmasterブランチのベースををupstream/masterにrebaseした
するとなにやら50個くらいのファイルでコンフリクトが発生していた

すでにいくつか手動でコンフリクトを解決していた状態

内容は自分はあまり身に覚えのない物ばかりなので、基本的にupstream/masterの内容を適用するようにしている
自分に身に覚えのないものとして、Babylonというディレクトリにbabylon.xxx.jsというファイルがクラスごとに生成されていた。これなんだろう?fork先のリポジトリにもGitHubを見る限りではなさそう。そしてGitKraken的には競合先が見当たらないと言われるのでファイルをひとつひとつ削除することに

にー兄さんにー兄さん

これ、一生手動でコンフリクトを解決しなくちゃいけないかもしれない
なぜかrebaseに一回成功しても次のコミットでまたコンフリクトする
これは無理だなと思った

repo cleanup、いったい何が起きたんだろうか……

にー兄さんにー兄さん

ということで惜しいけどfork先のリモートリポジトリごと消去してforkしなおすことにする
これは無理だと思ったので

fork先のリポジトリが消えるとPR消える......?そんなことはないとおもうんだけど、もしそうなったら怖いので
コミットの履歴を何とかしてバックアップしたいなぁ

にー兄さんにー兄さん

元のプルリクはもう破壊されてしまったのでここからたどるのは無理っぽい
なんでや......

にー兄さんにー兄さん

fork/feature/webxr-depth-sensing->fork/masterへの差分は普通に見ることができたので
一応変更は追えそう

にー兄さんにー兄さん

今のところGitHubにはforkを解除する方法はなくて、基本的にリポジトリを削除するしかない
ここの方法が良さそうだった

にー兄さんにー兄さん
  1. fork先を別のリポジトリにインポートする
  2. fork先を削除

という感じ。これなら差分のバックアップもできるね

にー兄さんにー兄さん

fork先をimportする方法は失敗した
かなり時間がかかってて心配だったけど、githubから失敗したよってメールが来た×2回

にー兄さんにー兄さん

それで次にupstreamのないローカルリポジトリをpushしようと思たが、こんな感じのエラーに
おそらく上限に引っかかったんだと思われる

にー兄さんにー兄さん

いやぁ難儀だなBabylonのリポジトリ
まぁ歴史があるし仕方がないのか……

にー兄さんにー兄さん

これ、詰みでは?
もうしょうがないから差分はローカルで見ることにするかぁ?

にー兄さんにー兄さん
webxr.d.ts
/**
 * BEGIN WebXR Depth Sensing Moudle
 * https://www.w3.org/TR/webxr-depth-sensing-1/
 */

type XRDepthUsage = "cpu-optimized" | "gpu-optimized";
type XRDepthDataFormat = "luminance-alpha" | "float32";

type XRDepthStateInit = {
    readonly usagePreference: XRDepthUsage[];
    readonly dataFormatPreference: XRDepthDataFormat[];
};

interface XRSessionInit {
    depthSensing?: XRDepthStateInit;
}

interface XRSession {
    readonly depthUsage: XRDepthUsage;
    readonly depthDataFormat: XRDepthDataFormat;
}

interface XRDepthInformation {
    readonly width: number;
    readonly height: number;

    readonly normDepthBufferFromNormView: XRRigidTransform;
    readonly rawValueToMeters: number;
}

interface XRCPUDepthInformation extends XRDepthInformation {
    readonly data: ArrayBuffer;

    getDepthInMeters(x: number, y: number): number;
}

interface XRFrame {
    getDepthInformation(view: XRView): ?XRCPUDepthInformation;
}

interface XRWebGLDepthInformation extends XRDepthInformation {
    readonly texture: WebGLTexture;
}

interface XRWebGLBinding {
    getDepthInformation(view: XRView): ?XRWebGLDepthInformation;
}

/**
 * BEGIN WebXR Depth Sensing Moudle
 * https://www.w3.org/TR/webxr-depth-sensing-1/
 */

1149行目から

にー兄さんにー兄さん
WebXRDepthSensing.ts
import { WebXRFeatureName, WebXRFeaturesManager } from "./../webXRFeaturesManager";
import { Observable } from "../../Misc/observable";
import type { WebXRSessionManager } from "../webXRSessionManager";
import { WebXRAbstractFeature } from "./WebXRAbstractFeature";
import { Tools } from "../../Misc/tools";

/**
 * Options for Depth Sensing feature
 */
export interface IWebXRDepthSensingOptions {
    /**
     *  The desired depth sensing usage for the session
     */
    usagePreference: XRDepthUsage[];
    /**
     * The desired depth sensing data format for the session
     */
    dataFormatPreference: XRDepthDataFormat[];
}

/**
 * WebXR Feature for WebXR Depth Sensing Module
 */
export class WebXRDepthSensing extends WebXRAbstractFeature {
    /**
     * An observable which triggered when the CPU depth information is acquired from XRFrame
     */
    public onCPUDepthInformationObservable: Observable<XRCPUDepthInformation> = new Observable();

    /**
     *An observable which triggered when the WebGL depth information is acquired from XRFrame
     */
    public onWebGLDepthInformationObservable: Observable<XRWebGLDepthInformation> = new Observable();

    /**
     * XRWebGLBinding which is used for acquiring WebGLDepthInformation
     */
    private _glBinding?: XRWebGLBinding;

    /**
     * The module's name
     */
    public static readonly Name = WebXRFeatureName.DEPTH_SENSING;

    /**
     * The (Babylon) version of this module.
     * This is an integer representing the implementation version.
     * This number does not correspond to the WebXR specs version
     */
    public static readonly Version = 1;

    /**
     * Creates a new instance of the depth sensing feature
     * @param _xrSessionManager the WebXRSessionManager
     * @param options options for WebXR Depth Sensing Feature
     */
    constructor(_xrSessionManager: WebXRSessionManager, public readonly options: IWebXRDepthSensingOptions) {
        super(_xrSessionManager);
        this.xrNativeFeatureName = "depth-sensing";

        // https://immersive-web.github.io/depth-sensing/
        Tools.Warn("depth-sensing is an experimental and unstable feature.");
    }

    /**
     * attach this feature
     * Will usually be called by the features manager
     *
     * @returns true if successful.
     */
    public attach(force?: boolean | undefined): boolean {
        if (!super.attach(force)) {
            return false;
        }

        const isDepthUsageAndFormatNull = this._xrSessionManager.session.depthDataFormat == null || this._xrSessionManager.session.depthUsage == null;
        if (isDepthUsageAndFormatNull) {
            return false;
        }

        this._glBinding = new XRWebGLBinding(this._xrSessionManager.session, this._xrSessionManager.scene.getEngine()._gl);

        return true;
    }

    /**
     * Dispose this feature and all of the resources attached
     */
    public dispose(): void {
        this.onCPUDepthInformationObservable.clear();
        this.onWebGLDepthInformationObservable.clear();
    }

    protected _onXRFrame(_xrFrame: XRFrame): void {
        const referenceSPace = this._xrSessionManager.referenceSpace;

        const pose = _xrFrame.getViewerPose(referenceSPace);
        if (pose == null) {
            return;
        }

        for (const view of pose.views) {
            const depthUsage = this._xrSessionManager.session.depthUsage;
            switch (depthUsage) {
                case "cpu-optimized":
                    this._getAndNotifyCPUDepthInformation(_xrFrame, view);
                    break;

                case "gpu-optimized":
                    if (this._glBinding == null) {
                        const context = this._xrSessionManager.scene.getEngine()._gl;
                        this._glBinding = new XRWebGLBinding(this._xrSessionManager.session, context);
                    }

                    this._getAndNotifyWebGLDepthInformation(_xrFrame, view, this._glBinding);
                    break;

                default:
                    Tools.Error("Unknown depth usage");
            }
        }
    }

    private _getAndNotifyCPUDepthInformation(frame: XRFrame, view: XRView): void {
        const cpuDepthInfo = frame.getDepthInformation(view);
        if (cpuDepthInfo == null) {
            return;
        }

        this.onCPUDepthInformationObservable.notifyObservers(cpuDepthInfo);
    }

    private _getAndNotifyWebGLDepthInformation(frame: XRFrame, view: XRView, glBinding: XRWebGLBinding): void {
        const webglDepthInfo = glBinding.getDepthInformation(view);
        if (webglDepthInfo == null) {
            return;
        }

        this.onWebGLDepthInformationObservable.notifyObservers(webglDepthInfo);
    }

    /**
     * Extends the session init object if needed
     * @returns augmentation object for the xr session init object.
     */
    public getXRSessionInitExtension(): Promise<Partial<XRSessionInit>> {
        const isDepthUsageDeclared = this.options.usagePreference != null && this.options.usagePreference.length !== 0;
        const isDataFormatDeclared = this.options.dataFormatPreference != null && this.options.dataFormatPreference.length !== 0;
        return new Promise((resolve) => {
            if (isDepthUsageDeclared && isDataFormatDeclared) {
                resolve({ depthSensing: this.options });
            } else {
                resolve({});
            }
        });
    }
}

WebXRFeaturesManager.AddWebXRFeature(
    WebXRDepthSensing.Name,
    (xrSessionManager, options) => {
        return () => new WebXRDepthSensing(xrSessionManager, options);
    },
    WebXRDepthSensing.Version,
    false
);

これはもう新規ファイルなのでこれだけ

にー兄さんにー兄さん

↑をfeaturesとしてexportする

export * from "./WebXRLightEstimation";
export * from "./WebXREyeTracking";
export * from "./WebXRWalkingLocomotion";
export * from "./WebXRDepthSensing";
export * from "./WebXRLayers";
にー兄さんにー兄さん
nativeXRFrame.ts
public getDepthInformation(view: XRView): XRCPUDepthInformation | null {
        return this._nativeImpl.getDepthInformation(view);
    }

にー兄さんにー兄さん

WebXRFeatureNameに登録

/**
     * The name of the depth sensing feature
     */
    public static readonly DEPTH_SENSING = "xr-depth-sensing";
にー兄さんにー兄さん

forkが消えても普通にPRとかは生きてるっぽいな
まぁそりゃそうなのかもだけど

にー兄さんにー兄さん

babylonのbuildSystem.mdにはこんな感じに書かれているので、これをやろうと思う
npm linkの-wオプションはワークスペースの指定っぽいんだよな
これを付ければcoreワークスペースだけlinkできるってことっぽい

にー兄さんにー兄さん

前やったときはビルドしているとなぜかVSCOdeの謎のプロセスが無限生成されてメモリが破壊されるというトラウマ的事態に遭遇したので、今回はどうか

にー兄さんにー兄さん

babylonjsの指示通りにnpx nx build coreしたあとにnpm link -w @babylonjs/coreしてみた
普通にできた
ちなみにnpm run prepublishOnly~~はコマンドがないといわれてできなかった

にー兄さんにー兄さん

そのごnpm viteで作ったvanilla-tsなプロジェクトにnpm link @babylonjs/coreしてみたらふつうに入った。すごい
ちゃんとsymlinkな表示にもなっている

にー兄さんにー兄さん

前はコード変更してビルドしなおしても反映されないということもあったけど、今回はなぜか普通に成功した
まぁなぜかというか本来こうあるべきなんだけど

にー兄さんにー兄さん

とりあえずPRクローズ前の状態まで持ってきた
link先のプロジェクトでもうまく型補完が効いているのでいい感じ

おそらくfeatureの有効化の処理はこれで正解な気がする
問題はWebXRDepthSensingの提供するAPIなので、これから底の部分を作りこんでいく

にー兄さんにー兄さん

上のフォーラム的には、engineでwrapなんちゃらを呼び出してWebGLTexture→InternalTextureに変換して
Textureのコンストラクタに指定すればできるよとのこと

InternalTexture→Textureなんだけど、Textureクラスにはそのような仕組みは用意されていないみたい
だけどBaseTextureのコンストラクタではいけそうなので、簡単にやるならBaeTextureかなぁ
https://doc.babylonjs.com/typedoc/classes/BABYLON.BaseTexture#constructor

BaseTextureはreadPixelsメソッドもあるし、マテリアルのテクスチャデータとしてもつかわれるのでこれはこれでありかなぁと

にー兄さんにー兄さん

cpuモードだった場合は16bit整数値配列なので、画素配列に変換してtextureを作る必要がある
RawTextureに便利メソッドがあるのでそれが使えそう

ArrayBufferViewが渡せるので、DepthInfo.dataをそのまま渡せるのかな

にー兄さんにー兄さん

ちょっと心配なのが16bitだったり32bit Floatだったりするのでどうなんだろう
まぁでもいいのか

にー兄さんにー兄さん

どういうデータであれ、textureからArrayBufferViewが取得できればいいかな
そういうAPIを探してみよう

にー兄さんにー兄さん

このあと

  • usageとformatごとにどういうやり方でテクスチャデータを生成するかを整理する
  • WebGLTexture→InternalTextureのAPIを調べる
にー兄さんにー兄さん

やっぱりなぜかデスクトップのVSCodeだと、rg.exeが無限に生成されてメモリ破壊されるわね......
https://twitter.com/ninisan_drumath/status/1612399466514087936

にー兄さんにー兄さん

これ、ノーパではできるので環境の際が原因だよなぁと思っていたのですが
もしかしたらvscodeの拡張機能が悪さしているのかなと思い、デスパのvscodeの拡張機能でいらないやつを消しまくってたら解決しました

なので結論どの拡張機能が悪さをしているのかはわからないけど、これはかなり良い進捗

にー兄さんにー兄さん

テクスチャ生成方法の整理をする
(CPU、GPU)×(luminance-alpha、float32)の4通り

CPUモード でluminance-alpha

WebXRCPUDepthInfoをXRFrameから取得
DepthInfo.dataから深度値配列(ArrayBufferView)を取得
RawTexture.CreateLuminanceAlphaTexture()でテクスチャを生成

CPUモードでfloat32

WebXRCPUDepthInfoをXRFrameから取得
DepthInfo.dataから深度値配列を取得
RawTexture.CreateRGBATextureでテクスチャを生成

GPUモードでluminancealpha・float32

WebGLDepthInfoをXRWebGLBindingから取得
WebGLTextureをInternalTextureに変換し、InternalTextureをコンストラクタ引数にBaseTextureを生成

にー兄さんにー兄さん

publicなAPIの整理をする

フィールド・プロパティ

name type 説明
width number | null depthデータのwidth。getter
height number | null 上のheight。getter
depthUsage WebXRDepthUsage | null cpuかgpuか。getter
dataFormat WebXRDepthDataFormat | null laかfloat32か getter
latestDepthImageTexture BaseTexture | null depth値を焼いたテクスチャ
latestWebGLTexture WebGLTexture | null GPUモードの場合に取得できるWebGLTexture
rawValueToMeters number | null DepthInfoから取得できるメートル換算の作用素
normDepthBufferFromNormView XRRigidTransform | null DepthInfoから取得できる値

メソッド

name args return 説明
ObtainLatestDepthBufferAsync void Promise<ArrayBufferView | null> 16bit整数値配列としてDepthバッファを取得する(非同期)
getDepthInMeters x:number, y:number number | null CPUモードの場合にUVからメートル単位のdepthを取得する
にー兄さんにー兄さん

キャッシュしたDepthInfoからAPIを露出させる作業していました
きれいな実装なのかちょっとわからないけど進めています
まぁもともとの実装を参考にしているので作業ゲー感はあります

にー兄さんにー兄さん

テクスチャの実装はやめに取り掛かりたいなぁ
そしてデバッグをしなければ

にー兄さんにー兄さん

デスパで久しぶりにビルドの時にメモリが破壊される奴に出会った
また苦戦していると、VSCodeのAuto Import拡張機能をdisableにしたら治った
マジか......

にー兄さんにー兄さん

そのあと色々やってたら、なぜかlinkしてもシンボリックリンクの先でディレクトリが空になってる自体に遭遇
たぶんビルド時にメモリリークの問題で中断したのが良く無くて、変な感じに残ったキャッシュのせいで
中途半端なキャッシュが読み込まれてそれが実行成功だと勘違いされていた可能性がある

そうしたらリンクは成功した

にー兄さんにー兄さん

そしたらなぜか次にDepthSensingFweatureだけビルドされていないという事態に陥った
普通に困った

にー兄さんにー兄さん

なぜかdevサーバーだとWebXRDepthSensingがexportされていないというシンタックスエラーがブラウザで出る
だけどビルドした後でnpm run previewしてみると普通に動いている。
この現象はいったい......

とはいえDepth sensingでデバッグできるまでになった~~~~

にー兄さんにー兄さん

ノーパソで作業
AndroidをワイヤレスデバッグでつないでWebView Debuggerでデバッグしてる
ノーパソではいい感じに動いている

にー兄さんにー兄さん

depth bufferからRawTextureを作成するやつ
widthとheightは設定できていそうだけど、readPixelsでバッファが取得できていない

にー兄さんにー兄さん

現状を整理

  • widthとかheightみたいな基本的ななデータはちゃんと取れていることがわかった
  • getDepthInMetersみたいな関数はXRFrameがアクティブでなくなってしまうために使用できない
  • DepthBufferはキャッシュできているがテクスチャの生成で失敗している
    • CreateLuminanceAlphaTextureの使い方(特にArrayBufferViewの中身)が違うのかもしれない
にー兄さんにー兄さん

このPlaygroundが参考になりそうな気がした
https://playground.babylonjs.com/#0NHRHE#0

にー兄さんにー兄さん

ちょっと変えてみてたんだけど、CreateRGBATextureの最後の引数にテクスチャタイプを指定できる
これでUNSIGNED_BYTEにしたら0-255の値でピクセル指定できた

にー兄さんにー兄さん
var texture = BABYLON.RawTexture.CreateRGBATexture(new Uint8Array(data), width, height, scene, false, false, BABYLON.Texture.NEAREST_SAMPLINGMODE, BABYLON.Engine.TEXTURETYPE_UNSIGNED_BYTE);
にー兄さんにー兄さん

戦略として、深度画像はRGBAで作る
その時にはUint8ArrayにArrayBufferを変換して使う
どんな画像になるかはわからないけども

にー兄さんにー兄さん

ということで16bitのdepthをRGBAに焼きこんだら邪悪なテクスチャができた!

深度画像とは違うんだけど、まぁテクスチャにすることはできた

にー兄さんにー兄さん

readPixelsはできないけど、depth値をモノクロで描画することはできるよなぁと思い、実装してみた
いい感じ

なおこれは誇張に表現しているので、実際のdepth画像はもっと黒っぽくなると思う

にー兄さんにー兄さん

RTextureで深度画像を生成

にー兄さんにー兄さん

これもまたdepth値を20くらいで割っている
256で割ったらほぼ真っ暗になってしまった

これはユーザが指定できるといいな

にー兄さんにー兄さん

Textureの生成処理を洗練させてきているフェーズ
GPUモードでデバッグできないのがちょっとな点だけど、割といい感じ

にー兄さんにー兄さん

やりたいことリスト

  • depth textureを作る際に圧縮する数値ファクタをユーザが指定できるようにする
  • テクスチャを生成するかどうかを指定できるようにする
  • getDepthInMeteersの実装
  • GPUモードでデバッグできないか検証
にー兄さんにー兄さん

depth textureを作る際に圧縮する数値ファクタをユーザが指定できるようにする

ここの要件を付けた理由として、普通にushortやFloatの最大値でマッピングしてdepth画像を表示すると、ほぼほぼ真っ黒なテクスチャになっちゃうということ(65mにたいして1mや3mはかなり小さい)

だからdepth値をマッピングするためにdepthの最大値をメートルかmm単位で指定できるようにしたい

にー兄さんにー兄さん

ちょっと時間が空いてしまいました
いい加減PR出したい気持ちになっています

なのであまり凝ったことはせずに、シンプルな方法で実装してメンテナの人の意見を聴こうかなーって思っているところです

にー兄さんにー兄さん

floatにすると色の限界を超えてもreadPixelsで値が保存されるっぽい

floatいいな
rawValueTometersかけてfkoatバッファで保存するのもあり

にー兄さんにー兄さん

CPUモードではRawTextureのバッファにfloatで記録するようにした
これはluminance-alphaでもfloat32でもどっちでも
rawValueToMetersをかけることによって統一している

floatにする利点は以下二つ

  • 表示上は2.55で打ち切られる→Depth画像としていい感じに映る
  • バッファ上は打ち切られることなく情報は残る
    • つまり表示上は3m以上は真っ赤になるけど、readPixelsすると3m以上の深度値も残ってるということ

実際に部屋の中で表示してみた結果がこちら

にー兄さんにー兄さん

これでDepthテクスチャの問題は解決
PR出す前にgetDepthInMetersの取得方法を自分なりに提示したい
こいつはメソッドだからかわからないけど、キャッシュしたDepthInfoから呼び出すとXRFrameが無効でエラーが出てしまう
つまり有効なXRFrameを取得して、そいつが生きている間じゃないと実行できないので
これはイベントで通知するのがいいと思っている

にー兄さんにー兄さん

今のところのAPI仕様

にー兄さんにー兄さん

一応説明用のコードも載せておく

  const depthSensing = featureManager.enableFeature(
    WebXRFeatureName.DEPTH_SENSING,
    "latest",
    {
      dataFormatPreference: ["luminance-alpha"],
      usagePreference: ["cpu-optimized"],
    } as IWebXRDepthSensingOptions,
    true,
    true
  ) as WebXRDepthSensing;

  depthSensing.onGetDepthInMetersAvailable.add((getDepthInMeters) => {
    const meters = getDepthInMeters(0.5, 0.5);
    console.log(meters);
  });

  sessionManager.onXRFrameObservable.add(() => {
    const {
      depthUsage,                  // "cpu-optimized" or "gpu-optimized"
      depthDataFormat,             // "luminance-alpha" or "float32"

      width,                       // depth image width
      height,                      // depth image height

      rawValueToMeters,            // operator of obtain depth value in meters

      normDepthBufferFromNormView, // An XRRigidTransform

      latestDepthImageTexture,     // RawTexture for depth image
      latestDepthBuffer,           // depth value array (cpu only)
      latestWebGLTexture,          // WebGLTexture of depth image (gpu only)
    } = depthSensing;

    // obtain depth image texture
    material.diffuseTexture = latestDepthImageTexture;
  });
にー兄さんにー兄さん

メンテナのRaananさんから8件のレビューをいただいた

にー兄さんにー兄さん

一件目はcachedWebGLTextureに関するもの
WebGLTextureを直接渡すのではなくInternalTextureに変更しようねということ

前にOpenしたPRでも同じことを言われたけど、なるべくBabylonで扱いやすい形式に変換しようというもの

にー兄さんにー兄さん

Just as a note - this is done so we can stay back-compat in case webxr will ever support webgpu

このように、後方互換性に言及されていたのでハッとしたけど
babylonjsはフレームワークとしてWebAPIとユーザコードの間にある抽象化レイヤーでもあるという面

つまり今回の場合、WebGLTextureではない何かが今後返ってくるようにDepthSensing Moduleが返すようになったとしても、ユーザコードは(理想的には)変更しなくても大丈夫という状況にしたい
そのためにはWebGLTextureをそのまま返すのではなく、Babylonで独自に作っている形式に変換するという方法が良いのだろう

にー兄さんにー兄さん

2件目 nit(細かい指摘)
nullのユニオンではなくNullableを返すようにする(これもBabylon独自の形式になる)

にー兄さんにー兄さん

3件目と4件目

これもBabylon独自の形式を返すようにするもの
DepthSensingが今後何かしらのアプデを以てネイティブな方を変更した場合にも耐えうる

にー兄さんにー兄さん

5件目
publicなgetterには対応するprivateなfieldを持たせておくこと
これは理由までは分からなかったけど、コードを追いやすくするためとか?
nitではあるので、あまり重要な意向ではなさそう

にー兄さんにー兄さん

7件目
これは確かに、毎フレーム呼び出される処理なので毎フレームエラーが出ることになり
それだったらもうdetachした方が良いよね ということでエラー文のあとにdetachすることで対応

にー兄さんにー兄さん

8件目
実はこれは自分の意図していたものではなかった
元々はWebAPIのネイティブAPIのための型定義なのかと思ってたら、どうやらBabylon Native用の定義だったらしい

でも現時点でBabylon Nativeには実装されていないのでエラーをthrowしましょうということになった

にー兄さんにー兄さん

パッと見、そんなに大きな指摘はなかったなという印象
それこそ一番最初のPRでは全体のAPI構成を変えなくてはいけない、大掛かりな変更になったが
今回は細かい指摘であったり、データ形式の変更などのものが多かった
なので割と早めに対応

にー兄さんにー兄さん

ここまで順調に対応していき、とうとうマージ!

スカッシュマージなのでコミット数は反映されないけど、確かに自分のコードが刻まれた瞬間だった

このスクラップは2023/03/08にクローズされました