Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source Control: The highlighted code is incorrect. #193266

Open
linbingquan opened this issue Sep 16, 2023 · 2 comments
Open

Source Control: The highlighted code is incorrect. #193266

linbingquan opened this issue Sep 16, 2023 · 2 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug diff-editor Diff editor mode issues
Milestone

Comments

@linbingquan
Copy link

linbingquan commented Sep 16, 2023

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.82.2
  • OS Version: Windows 11

Steps to Reproduce:

  1. Init a new project with git init
  2. Create file with code like before.js
  3. Run git add .
  4. Change the code like after.js
  5. Review the changes code in Source Control

The highlighted code is incorrect as shown by the arrow in the screenshot.

Code:

before.js

class RenderTarget {
	constructor(options = {}) {
		this.texture = new Texture(image, options.mapping, options.wrapS, options.wrapT, options.magFilter, options.minFilter, options.format, options.type, options.anisotropy, options.colorSpace);
		this.texture.isRenderTargetTexture = true;

		this.texture.flipY = false;
		this.texture.generateMipmaps = options.generateMipmaps !== undefined ? options.generateMipmaps : false;
		this.texture.internalFormat = options.internalFormat !== undefined ? options.internalFormat : null;
		this.texture.minFilter = options.minFilter !== undefined ? options.minFilter : LinearFilter;

		this.depthBuffer = options.depthBuffer !== undefined ? options.depthBuffer : true;
		this.stencilBuffer = options.stencilBuffer !== undefined ? options.stencilBuffer : false;

		this.depthTexture = options.depthTexture !== undefined ? options.depthTexture : null;

		this.samples = options.samples !== undefined ? options.samples : 0;
	}
}

after.js

class RenderTarget {
	constructor(options = {}) {
		const {
			generateMipmaps = false,
			internalFormat = null,
			minFilter = LinearFilter,
			depthBuffer = true,
			stencilBuffer = false,
			depthTexture = null,
			samples = 0
		} = options;

		this.texture = new Texture(image, options.mapping, options.wrapS, options.wrapT, options.magFilter, options.minFilter, options.format, options.type, options.anisotropy, options.colorSpace);
		this.texture.isRenderTargetTexture = true;

		this.texture.flipY = false;
		this.texture.generateMipmaps = generateMipmaps;
		this.texture.internalFormat = internalFormat;
		this.texture.minFilter = minFilter;

		this.depthBuffer = depthBuffer;
		this.stencilBuffer = stencilBuffer;

		this.depthTexture = depthTexture;

		this.samples = samples;
	}
}
--- a/test.js
+++ b/test.js
@@ -1,18 +1,28 @@
 class RenderTarget {
        constructor(options = {}) {
+               const {
+                       generateMipmaps = false,
+                       internalFormat = null,
+                       minFilter = LinearFilter,
+                       depthBuffer = true,
+                       stencilBuffer = false,
+                       depthTexture = null,
+                       samples = 0
+               } = options;
+
                this.texture = new Texture(image, options.mapping, options.wrapS, options.wrapT, options.magFilter, options.minFilter, options.format, options.type, options.anisotropy, options.colorSpace);
                this.texture.isRenderTargetTexture = true;

                this.texture.flipY = false;
-               this.texture.generateMipmaps = options.generateMipmaps !== undefined ? options.generateMipmaps : false;
-               this.texture.internalFormat = options.internalFormat !== undefined ? options.internalFormat : null;
-               this.texture.minFilter = options.minFilter !== undefined ? options.minFilter : LinearFilter;
+               this.texture.generateMipmaps = generateMipmaps;
+               this.texture.internalFormat = internalFormat;
+               this.texture.minFilter = minFilter;

-               this.depthBuffer = options.depthBuffer !== undefined ? options.depthBuffer : true;
-               this.stencilBuffer = options.stencilBuffer !== undefined ? options.stencilBuffer : false;
+               this.depthBuffer = depthBuffer;
+               this.stencilBuffer = stencilBuffer;

-               this.depthTexture = options.depthTexture !== undefined ? options.depthTexture : null;
+               this.depthTexture = depthTexture;

-               this.samples = options.samples !== undefined ? options.samples : 0;
+               this.samples = samples;
        }
 }

Screenshot:

图片

Relatived PR: mrdoob/three.js#26775

图片

@hediet
Copy link
Member

hediet commented Sep 26, 2023

Technically, the diff is not incorrect, but I still believe there is a bug in the diff shifting logic.

This is the result from the myers diff algorithm:

chrome_KWHUGoqZxl

Post processing yields this:

bMpIDMeFyR

(Playground repro)

@hediet hediet added bug Issue identified by VS Code Team member as probable bug diff-editor Diff editor mode issues labels Sep 26, 2023
@hediet hediet added this to the October 2023 milestone Sep 26, 2023
@hediet
Copy link
Member

hediet commented Sep 26, 2023

Related: #193342

@hediet hediet modified the milestones: October 2023, On Deck Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug diff-editor Diff editor mode issues
Projects
None yet
Development

No branches or pull requests

4 participants