Coder Social home page Coder Social logo

Comments (24)

twolfson avatar twolfson commented on July 29, 2024 1

We don't have this.image so we can't directly using their fix. I'm trying to understand what's going on now so I can see if we can still reuse it

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Would you be interested in opening a PR or providing a test image to reproduce/patch this?

from gif-encoder.

vegeta897 avatar vegeta897 commented on July 29, 2024

I would, but right now I'm trying to figure out another problem with transparency not working on frames after the first.

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Alright, I will sit tight for now. Thanks for the bug report and quick response =)

from gif-encoder.

vegeta897 avatar vegeta897 commented on July 29, 2024

The issue with transparency I'm running into is due to the palette generation creating duplicate entries of the same color. Because of this, the lookupRGB and findClosest functions may return different indexes (and will, more often than not, if most of the frame is transparent) and so the transparency index won't match the index that's actually used in the frame.

The palette generator is not only creating repeated entries but also including colors that aren't in the image at all. I'm guessing this is due to the compression aspect, for handling frames with >256 colors. These algorithms are too complex for me to dive into, so I'm going to resort to writing my own simple palette builder and deliberately keep my frames under 256 colors to avoid all these issues.

Sorry for not being able to help further.

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Can you at least provide the source images or a test case to reproduce with? =/

from gif-encoder.

vegeta897 avatar vegeta897 commented on July 29, 2024

No source images, I'm using a canvas. Here's an example:

var Canvas = require('canvas');
var GIFEncoder = require('gifencoder');
var fs = require('fs');
var canvas = new Canvas(100, 100),
    ctx = canvas.getContext('2d');

var encoder = new GIFEncoder(100,100);
var stream = encoder.createReadStream();
stream.pipe(fs.createWriteStream(__dirname + '/test.gif'));
encoder.start();
encoder.setRepeat(0);
encoder.setDelay(50);
encoder.setTransparent(0xFF00FF);
for(var i = 0; i < 10; i++) {
    ctx.fillStyle = '#FF00FF';
    ctx.fillRect(0,0,100,100);
    ctx.fillStyle = '#000000';
    ctx.fillRect(i*10,40,10,10);
    encoder.addFrame(ctx);
}
encoder.finish();

Result with unmodified library:
test3

Result with my initial fix, only the first frame gets working transparency:
test

Result with my simple <256 color palette generator, working as intended:
test2

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Awesome, thanks for the test case =)

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

I will take a look at this by the end of next weekend but maybe much sooner (e.g. today, tomorrow).

from gif-encoder.

vegeta897 avatar vegeta897 commented on July 29, 2024

Cool! No urgency from me. I will likely stick with my palette generator unless you change that too so it doesn't generate unused or repeated colors.

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Taking a look at this now

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Ahhh, I think you have wrong repo =(

You used gifencoder as the require as well as encoder.createReadStream(). This repo is gif-encoder and doesn't have createReadStream as a method =/ I believe you are looking for:

https://github.com/eugeneware/gifencoder

from gif-encoder.

vegeta897 avatar vegeta897 commented on July 29, 2024

Well that's embarrassing 😛

Sorry for the trouble. Maybe I'll write a test case for this lib when I have the time, I believe the same method in yours will have the same issue, as well as the palette stuff.

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Ah, I think you might be right. Reopening this PR as something to triage

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Already moved onto another task though, so going to take a look at this later =/

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

I was able to reproduce the issue. I modularized the script a bit and modified it for our needs.

// https://github.com/twolfson/gif-encoder/issues/5
var Canvas = require('canvas');
var GIFEncoder = require('./');
var fs = require('fs');
var WIDTH = 100;
var HEIGHT = 100;
var STEP = 10;
var canvas = new Canvas(WIDTH, HEIGHT),
    ctx = canvas.getContext('2d');

var encoder = new GIFEncoder(WIDTH, HEIGHT);
encoder.pipe(fs.createWriteStream(__dirname + '/test.gif'));
encoder.setRepeat(0);
encoder.setDelay(50);
encoder.setTransparent(0xFF00FF);
encoder.writeHeader();
for(var i = 0; i < STEP; i++) {
    ctx.fillStyle = '#FF00FF';
    ctx.fillRect(0,0, WIDTH, HEIGHT);
    ctx.fillStyle = '#000000';
    ctx.fillRect(i * WIDTH / STEP, 4 * HEIGHT / STEP, WIDTH / STEP, HEIGHT / STEP);
    encoder.addFrame(ctx.getImageData(0, 0, WIDTH, HEIGHT).data);
}
encoder.finish();

Unfortunately the line 302 fix seems to break things more (e.g. the square goes hollow at some point) for me.

Since we don't have a full fix and this seems to be a deep encoding problem, I'm prob going to sit tight to see what gifencoder does =/

Thanks for the bug report =)

from gif-encoder.

vegeta897 avatar vegeta897 commented on July 29, 2024

Sounds reasonable to me, thanks for the correspondence.

from gif-encoder.

scinos avatar scinos commented on July 29, 2024

gifencoder has this PR pending eugeneware/gifencoder#5, which seems to fix the problem (more or less) for me.

(disclaimer: I'm generating the GIF from a buffer of pixels, not from canvas). If you are still interested in fixing this I can open a PR with the change (or ask the original PR author to apply it to this repo instead)

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Yep, we are aware of that PR -- it was linked by @vegeta897 on their issue on eugeneware/gifencoder#7

I think we expected the PR to be merged/closed by now but I guess not

I don't think I ever tried the fix on the script we wrote before. Going to try it now

from gif-encoder.

scinos avatar scinos commented on July 29, 2024

This is the hack I did:

  // get closest match to transparent color if specified
  if (this.transparent !== null) {
    this.transIndex = this.findClosest(this.transparent);

    var transR = (this.transparent & 0xFF0000) >> 16;
    var transG = (this.transparent & 0x00FF00) >> 8;
    var transB = (this.transparent & 0x0000FF);

    for (var pixelIndex = 0; pixelIndex < nPix; pixelIndex++) {
      if (
        this.pixels[pixelIndex * 3] == transR &&
        this.pixels[pixelIndex * 3 + 1] == transG &&
        this.pixels[pixelIndex * 3 + 2] == transB
      ) {
        this.indexedPixels[pixelIndex] = this.transIndex;
      }
    }
  }

Instead of looking for original pixels with full alpha, I look for pixels with the same RGB values than the transparent color.

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

But the transparent color is still a color so it should already be in indexedPixels from:

  var k = 0;
  for (var j = 0; j < nPix; j++) {
    var index = imgq.lookupRGB(
      this.pixels[k++] & 0xff,
      this.pixels[k++] & 0xff,
      this.pixels[k++] & 0xff
    );
    this.usedEntry[index] = true;
    this.indexedPixels[j] = index;
  }

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

The this.findClosest is attempting to find the already indexed matching color =/

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

Alright, I don't really have time tonight to look at this further =/ I do think there are bugs in 2 places:

  • findClosest is broken -- it should be using the var index = i / 3 fix
  • We shouldn't be sending transparency byte when it isn't set

However, the transparency issue is eluding me =(

from gif-encoder.

twolfson avatar twolfson commented on July 29, 2024

This has been repaired by @scinos in #7 and released in 0.6.0

from gif-encoder.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.