Skip to content

Conversation

chanayane
Copy link
Contributor

I recently noticed that in my self-compiled viewers, all my uploads looked very bad, with a lot of compression artifacts. The greater the dimensions of the texture, the ugliest it looked, like, over-compressed JPEGs.

I do not have a KDU license, so I have to use the OpenJPEG implementation in my viewer. This affected the uploading of textures and the saving of snapshots to inventory.

This bothered me a lot so I started to investigate the issue.

I discovered that the culprit was indeed the OpenJPEG implementation. The maximum size for all encoded images was set to 32KB. So 1024x1024 and especially 2048x2048 textures looked really, really bad (as good as a 32KB 2048x2048 texture can look...).

The bad line:
parameters.max_cs_size = (1 << 15);

This was set in the constructor of the JPEG2KEncode class and applied to all pictures regardless of their dimensions.
The documentations says the following for max_cs_size: Maximum size (in bytes) for the whole codestream. If == 0, codestream size limitation is not considered. If it does not comply with tcp_rates, max_cs_size prevails and a warning is issued.

Also the number of layers was hardcoded to 5, which seemed wrong to me, it should depend on the surface of the texture.

My fix is:

  1. Computing the number of layers depending on image surface, much like what is done in the KDU implementation. So I moved the definition of num_layers and tcp_rates values to the encode method.
  2. Defining the rates according to the number of layers, the last one gets set to 1 / DEFAULT_COMPRESSION_RATE instead of the fixed 30.0f to avoid reducing quality of the topmost layer, and each previous layer multiplies its subsequent layer with a multiplier (I used 15, 4, 2... to be close to the previous defaults of 120, 480, 960 and 1920)
  3. Computing max_cs_size: for each layer, multiply the surface by the number of components and by 1/layer rate and add the result to max_cs_size

So a big texture would be allowed more bytes and more layers than a tiny one.

After the fix, the quality of the uploaded textures has improved greatly. In some cases I can still see some very minor compression artefacts and blurriness but they are not too noticeable.

Copy link
Contributor

@marchcat marchcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution!

Pre-commit check is unhappy about trailing whitespaces; the suggested changes should fix that. Please see secondlife/git-hooks for more info.

@marchcat
Copy link
Contributor

@chanayane, it looks like I can't directly apply those suggestions to your repo. Please fix them so we could proceed.

@marchcat
Copy link
Contributor

raw diff:

diff --git a/indra/llimagej2coj/llimagej2coj.cpp b/indra/llimagej2coj/llimagej2coj.cpp
index a035c1c..f4bcb97 100644
--- a/indra/llimagej2coj/llimagej2coj.cpp
+++ b/indra/llimagej2coj/llimagej2coj.cpp
@@ -437,7 +437,7 @@ public:
             parameters.max_cs_size = 0; // do not limit size for reversible compression
             parameters.irreversible = 0; // should be the default, but, just in case
             parameters.tcp_numlayers = 1;
-            /* documentation seems to be wrong, should be 0.0f for lossless, not 1.0f 
+            /* documentation seems to be wrong, should be 0.0f for lossless, not 1.0f
                see https://github.com/uclouvain/openjpeg/blob/39e8c50a2f9bdcf36810ee3d41bcbf1cc78968ae/src/lib/openjp2/j2k.c#L7755
             */
             parameters.tcp_rates[0] = 0.0f;
@@ -538,7 +538,7 @@ public:
 
             //ensure that we have at least a minimal size
             max_cs_size = llmax(max_cs_size, (U32)FIRST_PACKET_SIZE);
-           
+
             parameters.max_cs_size = max_cs_size;
         }
 

@chanayane
Copy link
Contributor Author

@chanayane, it looks like I can't directly apply those suggestions to your repo. Please fix them so we could proceed.

Hello @marchcat, thank you for your review, I made the suggested changes.

@chanayane chanayane force-pushed the pr-fix-bad-upload-quality-with-openjpeg branch from 42ed648 to 11acae9 Compare July 18, 2024 06:56
@marchcat
Copy link
Contributor

Will merge this after #2060, which should fix the pre-commit failure.

@chanayane chanayane force-pushed the pr-fix-bad-upload-quality-with-openjpeg branch from 11acae9 to 4d6d9ea Compare July 18, 2024 07:43
@marchcat marchcat merged commit 87b5ecf into secondlife:develop Jul 18, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants