Image Empty: Wrong aspect ratio if Texture Size Limit is used #65812

New Issue

Philip Holzmann · 2019-06-14T19:05:40+02:00

Philip Holzmann commented

2019-06-14 19:05:40 +02:00

System Information
Operating system: Windows 10 64 bit
Graphics card: Nvidia GTX 750 Ti

Blender Version
Broken: blender-2.80.0-git.81b68f7279cd-windows64
Working: 2.79

Short description of error
Reference Images have the wrong aspect ratio, if the Limit Size in the user preferences is set lower than the longest side of the image.

Exact steps for others to reproduce the error
LimitTextureSize.blend
Set the Limit Size in the user preferences, under Viewport > Textures to a value greater or less than 1024.
With values greater or equal to 1024, the reference image is rectangular.
With values less than 1024 the image becomes square.

Optionally, file working in Blender 2.79
texture-size-limit.blend

**System Information** Operating system: Windows 10 64 bit Graphics card: Nvidia GTX 750 Ti **Blender Version** Broken: blender-2.80.0-git.81b68f7279cd-windows64 Working: 2.79 **Short description of error** Reference Images have the wrong aspect ratio, if the *Limit Size* in the user preferences is set lower than the longest side of the image. **Exact steps for others to reproduce the error** [LimitTextureSize.blend](https://archive.blender.org/developer/F7103038/LimitTextureSize.blend) Set the *Limit Size* in the user preferences, under Viewport > Textures to a value greater or less than 1024. With values greater or equal to 1024, the reference image is rectangular. With values less than 1024 the image becomes square. **Optionally, file working in Blender 2.79** [texture-size-limit.blend](https://archive.blender.org/developer/F7103090/texture-size-limit.blend)

Philip Holzmann commented

2019-06-14 19:05:40 +02:00

Added subscriber: @Foaly

Dalai Felinto changed title from ~~Reference Image Aspect Ratio Wrong If Texture Size Limit Is Used~~ to Image Empty: Wrong aspect ratio if Texture Size Limit is used

2019-06-14 20:31:02 +02:00

Dalai Felinto commented

2019-06-14 20:31:51 +02:00

Added subscriber: @dfelinto

Dalai Felinto commented

2019-06-14 20:31:51 +02:00

Confirmed. And in 2.79 it was working fine.

Dalai Felinto commented

2019-06-14 20:52:03 +02:00

Actually although 2.79 doesn't have this issue, it is clearly not reducing the texture limit, which is why this works.

Actually although 2.79 doesn't have *this* issue, it is clearly not reducing the texture limit, which is why this works.

Campbell Barton was assigned by Dalai Felinto

2019-06-14 21:07:36 +02:00

Dalai Felinto commented

2019-06-14 21:07:36 +02:00

Added subscriber: @ideasman42

Dalai Felinto commented

2019-06-14 21:07:36 +02:00

@ideasman42 we have a problem here.

Manually reverting 9e82499d2d (i.e., P1005) would fix this, but the original bug (#59347) comes back.

@ideasman42 we have a problem here. Manually reverting 9e82499d2d (i.e., [P1005](https://archive.blender.org/developer/P1005.txt)) would fix this, but the original bug (#59347) comes back.

Campbell Barton commented

2019-06-21 09:51:34 +02:00

It's not great that we cant access the size of a loaded texture without freeing the previously loaded textures.
AFAICS supporting this would mean larger changes to the image API.

Looked into storing the original size of the GPUTexture before it's clamped.

Here are two patches that do this, I'm thinking the first one is probably better since:

It doesn't store more data in DNA.
It doesn't risk values becoming out-of-sync.
P1006: #65812 possible fix (store original size in the GPUTexture)

diff --git a/source/blender/draw/modes/object_mode.c b/source/blender/draw/modes/object_mode.c
index e8c600ee545..1fb56f2f072 100644
--- a/source/blender/draw/modes/object_mode.c
+++ b/source/blender/draw/modes/object_mode.c
@@ -979,8 +979,8 @@ static void DRW_shgroup_empty_image(OBJECT_Shaders *sh_data,
   if (ima != NULL) {
     tex = GPU_texture_from_blender(ima, ob->iuser, GL_TEXTURE_2D);
     if (tex) {
-      size- [x] = GPU_texture_width(tex);
-      size- [x] = GPU_texture_height(tex);
+      size- [x] = GPU_texture_orig_width(tex);
+      size- [x] = GPU_texture_orig_height(tex);
     }
   }
 
diff --git a/source/blender/gpu/GPU_texture.h b/source/blender/gpu/GPU_texture.h
index 3fb7dfc6331..c798788724f 100644
--- a/source/blender/gpu/GPU_texture.h
+++ b/source/blender/gpu/GPU_texture.h
@@ -237,6 +237,9 @@ int GPU_texture_detach_framebuffer(GPUTexture *tex, struct GPUFrameBuffer *fb);
 int GPU_texture_target(const GPUTexture *tex);
 int GPU_texture_width(const GPUTexture *tex);
 int GPU_texture_height(const GPUTexture *tex);
+int GPU_texture_orig_width(const GPUTexture *tex);
+int GPU_texture_orig_height(const GPUTexture *tex);
+void GPU_texture_orig_size_set(GPUTexture *tex, int w, int h);
 int GPU_texture_layers(const GPUTexture *tex);
 eGPUTextureFormat GPU_texture_format(const GPUTexture *tex);
 int GPU_texture_samples(const GPUTexture *tex);
diff --git a/source/blender/gpu/intern/gpu_draw.c b/source/blender/gpu/intern/gpu_draw.c
index 7813ae68371..e48f14a6894 100644
--- a/source/blender/gpu/intern/gpu_draw.c
+++ b/source/blender/gpu/intern/gpu_draw.c
@@ -469,6 +469,9 @@ GPUTexture *GPU_texture_from_blender(Image *ima, ImageUser *iuser, int textarget
   BKE_image_release_ibuf(ima, ibuf, NULL);
 
   *tex = GPU_texture_from_bindcode(textarget, bindcode);
+
+  GPU_texture_orig_size_set(*tex, ibuf->x, ibuf->y);
+
   return *tex;
 }
 
diff --git a/source/blender/gpu/intern/gpu_texture.c b/source/blender/gpu/intern/gpu_texture.c
index 58d0dd5576f..54e93c361ca 100644
--- a/source/blender/gpu/intern/gpu_texture.c
+++ b/source/blender/gpu/intern/gpu_texture.c
@@ -68,6 +68,7 @@ typedef enum eGPUTextureFormatFlag {
 /* GPUTexture */
 struct GPUTexture {
   int w, h, d;        /* width/height/depth */
+  int orig_w, orig_h; /* width/height (of source data), optional. */
   int number;         /* number for multitexture binding */
   int refcount;       /* reference count */
   GLenum target;      /* GL_TEXTURE_* */
@@ -1778,6 +1779,22 @@ int GPU_texture_height(const GPUTexture *tex)
   return tex->h;
 }
 
+int GPU_texture_orig_width(const GPUTexture *tex)
+{
+  return tex->orig_w;
+}
+
+int GPU_texture_orig_height(const GPUTexture *tex)
+{
+  return tex->orig_h;
+}
+
+void GPU_texture_orig_size_set(GPUTexture *tex, int w, int h)
+{
+  tex->orig_w = w;
+  tex->orig_h = h;
+}
+
 int GPU_texture_layers(const GPUTexture *tex)
 {
   return tex->d;

P1007: #65812 possible fix (store original size in the Image ~ DNA)

diff --git a/source/blender/draw/modes/object_mode.c b/source/blender/draw/modes/object_mode.c
index e8c600ee545..1ee23c95d38 100644
--- a/source/blender/draw/modes/object_mode.c
+++ b/source/blender/draw/modes/object_mode.c
@@ -979,8 +979,9 @@ static void DRW_shgroup_empty_image(OBJECT_Shaders *sh_data,
   if (ima != NULL) {
     tex = GPU_texture_from_blender(ima, ob->iuser, GL_TEXTURE_2D);
     if (tex) {
-      size- [x] = GPU_texture_width(tex);
-      size- [x] = GPU_texture_height(tex);
+      const int *gpu_size = ima->gputexture_orig_size[TEXTARGET_TEXTURE_2D];
+      size- [x] = gpu_size[0];
+      size- [x] = gpu_size[1];
     }
   }
 
diff --git a/source/blender/gpu/intern/gpu_draw.c b/source/blender/gpu/intern/gpu_draw.c
index 7813ae68371..8a7f9ade969 100644
--- a/source/blender/gpu/intern/gpu_draw.c
+++ b/source/blender/gpu/intern/gpu_draw.c
@@ -196,16 +196,15 @@ float GPU_get_anisotropic(void)
 
 /* Set OpenGL state for an MTFace */
 
-static GPUTexture **gpu_get_image_gputexture(Image *ima, GLenum textarget)
+static int gpu_get_index_from_textarget(GLenum textarget)
 {
   if (textarget == GL_TEXTURE_2D) {
-    return &ima->gputexture[TEXTARGET_TEXTURE_2D];
+    return TEXTARGET_TEXTURE_2D;
   }
   else if (textarget == GL_TEXTURE_CUBE_MAP) {
-    return &ima->gputexture[TEXTARGET_TEXTURE_CUBE_MAP];
+    return TEXTARGET_TEXTURE_CUBE_MAP;
   }
-
-  return NULL;
+  return -1;
 }
 
 static uint gpu_texture_create_from_ibuf(Image *ima, ImBuf *ibuf, int textarget)
@@ -443,12 +442,23 @@ GPUTexture *GPU_texture_from_blender(Image *ima, ImageUser *iuser, int textarget
     ima->gpuflag &= ~IMA_GPU_REFRESH;
   }
 
+  const int tex_index = gpu_get_index_from_textarget(textarget);
+  if (tex_index == -1) {
+    BLI_assert(0);
+    return NULL;
+  }
+
   /* Test if we already have a texture. */
-  GPUTexture **tex = gpu_get_image_gputexture(ima, textarget);
+  GPUTexture **tex = &(ima->gputexture[tex_index]);
   if (*tex) {
     return *tex;
   }
 
+  int *gputexture_orig_size = ima->gputexture_orig_size[tex_index];
+
+  gputexture_orig_size- [x] = 0;
+  gputexture_orig_size- [x] = 0;
+
   /* Check if we have a valid image. If not, we return a dummy
    * texture with zero bindcode so we don't keep trying. */
   uint bindcode = 0;
@@ -469,6 +479,10 @@ GPUTexture *GPU_texture_from_blender(Image *ima, ImageUser *iuser, int textarget
   BKE_image_release_ibuf(ima, ibuf, NULL);
 
   *tex = GPU_texture_from_bindcode(textarget, bindcode);
+
+  gputexture_orig_size- [x] = ibuf->x;
+  gputexture_orig_size- [x] = ibuf->y;
+
   return *tex;
 }
 
diff --git a/source/blender/makesdna/DNA_image_types.h b/source/blender/makesdna/DNA_image_types.h
index 5e4ed16d28e..6b4bcec6057 100644
--- a/source/blender/makesdna/DNA_image_types.h
+++ b/source/blender/makesdna/DNA_image_types.h
@@ -112,6 +112,9 @@ typedef struct Image {
   /** Not written in file 2 = TEXTARGET_COUNT. */
   struct GPUTexture *gputexture[2];
 
+  /** 2 = TEXTARGET_COUNT. */
+  int gputexture_orig_size- [x][2];
+
   /* sources from: */
   ListBase anims;
   struct RenderResult *rr;

It's not great that we cant access the size of a loaded texture without freeing the previously loaded textures. AFAICS supporting this would mean larger changes to the image API. Looked into storing the original size of the GPUTexture before it's clamped. Here are two patches that do this, I'm thinking the first one is probably better since: - It doesn't store more data in DNA. - It doesn't risk values becoming out-of-sync. - [P1006: #65812 possible fix (store original size in the GPUTexture)](https://archive.blender.org/developer/P1006.txt) ``` diff --git a/source/blender/draw/modes/object_mode.c b/source/blender/draw/modes/object_mode.c index e8c600ee545..1fb56f2f072 100644 --- a/source/blender/draw/modes/object_mode.c +++ b/source/blender/draw/modes/object_mode.c @@ -979,8 +979,8 @@ static void DRW_shgroup_empty_image(OBJECT_Shaders *sh_data, if (ima != NULL) { tex = GPU_texture_from_blender(ima, ob->iuser, GL_TEXTURE_2D); if (tex) { - size- [x] = GPU_texture_width(tex); - size- [x] = GPU_texture_height(tex); + size- [x] = GPU_texture_orig_width(tex); + size- [x] = GPU_texture_orig_height(tex); } } diff --git a/source/blender/gpu/GPU_texture.h b/source/blender/gpu/GPU_texture.h index 3fb7dfc6331..c798788724f 100644 --- a/source/blender/gpu/GPU_texture.h +++ b/source/blender/gpu/GPU_texture.h @@ -237,6 +237,9 @@ int GPU_texture_detach_framebuffer(GPUTexture *tex, struct GPUFrameBuffer *fb); int GPU_texture_target(const GPUTexture *tex); int GPU_texture_width(const GPUTexture *tex); int GPU_texture_height(const GPUTexture *tex); +int GPU_texture_orig_width(const GPUTexture *tex); +int GPU_texture_orig_height(const GPUTexture *tex); +void GPU_texture_orig_size_set(GPUTexture *tex, int w, int h); int GPU_texture_layers(const GPUTexture *tex); eGPUTextureFormat GPU_texture_format(const GPUTexture *tex); int GPU_texture_samples(const GPUTexture *tex); diff --git a/source/blender/gpu/intern/gpu_draw.c b/source/blender/gpu/intern/gpu_draw.c index 7813ae68371..e48f14a6894 100644 --- a/source/blender/gpu/intern/gpu_draw.c +++ b/source/blender/gpu/intern/gpu_draw.c @@ -469,6 +469,9 @@ GPUTexture *GPU_texture_from_blender(Image *ima, ImageUser *iuser, int textarget BKE_image_release_ibuf(ima, ibuf, NULL); *tex = GPU_texture_from_bindcode(textarget, bindcode); + + GPU_texture_orig_size_set(*tex, ibuf->x, ibuf->y); + return *tex; } diff --git a/source/blender/gpu/intern/gpu_texture.c b/source/blender/gpu/intern/gpu_texture.c index 58d0dd5576f..54e93c361ca 100644 --- a/source/blender/gpu/intern/gpu_texture.c +++ b/source/blender/gpu/intern/gpu_texture.c @@ -68,6 +68,7 @@ typedef enum eGPUTextureFormatFlag { /* GPUTexture */ struct GPUTexture { int w, h, d; /* width/height/depth */ + int orig_w, orig_h; /* width/height (of source data), optional. */ int number; /* number for multitexture binding */ int refcount; /* reference count */ GLenum target; /* GL_TEXTURE_* */ @@ -1778,6 +1779,22 @@ int GPU_texture_height(const GPUTexture *tex) return tex->h; } +int GPU_texture_orig_width(const GPUTexture *tex) +{ + return tex->orig_w; +} + +int GPU_texture_orig_height(const GPUTexture *tex) +{ + return tex->orig_h; +} + +void GPU_texture_orig_size_set(GPUTexture *tex, int w, int h) +{ + tex->orig_w = w; + tex->orig_h = h; +} + int GPU_texture_layers(const GPUTexture *tex) { return tex->d; ``` - [P1007: #65812 possible fix (store original size in the Image ~ DNA)](https://archive.blender.org/developer/P1007.txt) ``` diff --git a/source/blender/draw/modes/object_mode.c b/source/blender/draw/modes/object_mode.c index e8c600ee545..1ee23c95d38 100644 --- a/source/blender/draw/modes/object_mode.c +++ b/source/blender/draw/modes/object_mode.c @@ -979,8 +979,9 @@ static void DRW_shgroup_empty_image(OBJECT_Shaders *sh_data, if (ima != NULL) { tex = GPU_texture_from_blender(ima, ob->iuser, GL_TEXTURE_2D); if (tex) { - size- [x] = GPU_texture_width(tex); - size- [x] = GPU_texture_height(tex); + const int *gpu_size = ima->gputexture_orig_size[TEXTARGET_TEXTURE_2D]; + size- [x] = gpu_size[0]; + size- [x] = gpu_size[1]; } } diff --git a/source/blender/gpu/intern/gpu_draw.c b/source/blender/gpu/intern/gpu_draw.c index 7813ae68371..8a7f9ade969 100644 --- a/source/blender/gpu/intern/gpu_draw.c +++ b/source/blender/gpu/intern/gpu_draw.c @@ -196,16 +196,15 @@ float GPU_get_anisotropic(void) /* Set OpenGL state for an MTFace */ -static GPUTexture **gpu_get_image_gputexture(Image *ima, GLenum textarget) +static int gpu_get_index_from_textarget(GLenum textarget) { if (textarget == GL_TEXTURE_2D) { - return &ima->gputexture[TEXTARGET_TEXTURE_2D]; + return TEXTARGET_TEXTURE_2D; } else if (textarget == GL_TEXTURE_CUBE_MAP) { - return &ima->gputexture[TEXTARGET_TEXTURE_CUBE_MAP]; + return TEXTARGET_TEXTURE_CUBE_MAP; } - - return NULL; + return -1; } static uint gpu_texture_create_from_ibuf(Image *ima, ImBuf *ibuf, int textarget) @@ -443,12 +442,23 @@ GPUTexture *GPU_texture_from_blender(Image *ima, ImageUser *iuser, int textarget ima->gpuflag &= ~IMA_GPU_REFRESH; } + const int tex_index = gpu_get_index_from_textarget(textarget); + if (tex_index == -1) { + BLI_assert(0); + return NULL; + } + /* Test if we already have a texture. */ - GPUTexture **tex = gpu_get_image_gputexture(ima, textarget); + GPUTexture **tex = &(ima->gputexture[tex_index]); if (*tex) { return *tex; } + int *gputexture_orig_size = ima->gputexture_orig_size[tex_index]; + + gputexture_orig_size- [x] = 0; + gputexture_orig_size- [x] = 0; + /* Check if we have a valid image. If not, we return a dummy * texture with zero bindcode so we don't keep trying. */ uint bindcode = 0; @@ -469,6 +479,10 @@ GPUTexture *GPU_texture_from_blender(Image *ima, ImageUser *iuser, int textarget BKE_image_release_ibuf(ima, ibuf, NULL); *tex = GPU_texture_from_bindcode(textarget, bindcode); + + gputexture_orig_size- [x] = ibuf->x; + gputexture_orig_size- [x] = ibuf->y; + return *tex; } diff --git a/source/blender/makesdna/DNA_image_types.h b/source/blender/makesdna/DNA_image_types.h index 5e4ed16d28e..6b4bcec6057 100644 --- a/source/blender/makesdna/DNA_image_types.h +++ b/source/blender/makesdna/DNA_image_types.h @@ -112,6 +112,9 @@ typedef struct Image { /** Not written in file 2 = TEXTARGET_COUNT. */ struct GPUTexture *gputexture[2]; + /** 2 = TEXTARGET_COUNT. */ + int gputexture_orig_size- [x][2]; + /* sources from: */ ListBase anims; struct RenderResult *rr; ```

Dalai Felinto commented

2019-06-22 02:24:47 +02:00

First patch seems less intrusive as well +1 for it imho.

blender-admin commented

2019-06-24 14:09:04 +02:00

This issue was referenced by a699a9680b

This issue was referenced by a699a9680bf88cef76fcf6be89d62bdeb532bd4c

Campbell Barton commented

2019-06-24 14:11:44 +02:00

Changed status from 'Open' to: 'Resolved'

Campbell Barton closed this issue

2019-06-24 14:11:44 +02:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Image Empty: Wrong aspect ratio if Texture Size Limit is used #65812