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

Saving a PGraphics results in black image 50% of the time #4578

Closed
AmnonOwed opened this Issue Jul 16, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@AmnonOwed
Contributor

AmnonOwed commented Jul 16, 2016

Saving PGraphics appears to be unreliable in Processing 3.1.1. However, since the chance of failure is lower than 100%, it took some time to track down the exact issue.

I have created a test sketch that creates a PGraphics 7 times and saves each created PGraphics 3 times. So a total of 21 images are saved. When I run this test sketch, results vary, but on average about 50% of the saved PGraphics are incorrect, meaning they are fully black instead of the actual image. It really is a game of chance, so which ones are incorrect differs each time. However once the initial PGraphics is created 'wrong', then all saves are black. But in the P2D renderer, even when the first save of a created PGraphics is succesful, there is still a chance that subsequent saves might fail.

Additional findings:

  • The problem occurs in Processing 3.1.1 (it does not occur in Processing 2.2.1)
  • It is not caused by smooth (in the test sketch smooth is not even used)
  • It is not caused by too large PGraphics dimensions (the test sketch' PGraphics dimensions are very small)
  • It may be related to multiple instantiations, especially when the dimensions vary (when the PGraphics dimensions of subsequent instantiations are the same, the problem does not occur)
  • It is not only OpenGL-related (the problem occurs in both the JAVA2D and P2D renderers)
  • The problem occurs when similar code is run from keyPressed, setup() or draw().
  • Displaying the created PGraphics seems to work consistently, but even after correct display, saving may fail.
  • There is no indication of a problem, apart from the black image, so there are no NPE's, error messages or anything to indicate saving was unsuccessful. Literally the saving WAS succesful (there is a readable image file), except it just doesn't show the actual image that was in the PGraphics.

System specs: Win 8 64-bit. NVIDIA GeForce GTX 770

Example Sketch:

PGraphics pg;

void setup() {
  size(200, 200, P2D);

  for (int i=0; i<7; i++) {
    createCanvas(100 + i * 10);
    for (int j=0; j<3; j++) {
      saveCanvas("canvas" + i  + "-save" + j);
    }
  }
  exit();
}

void createCanvas(int d) {
  // create PGraphics
  pg = createGraphics(d, d, P2D);

  // draw to PGraphics
  pg.beginDraw();
  pg.background(255, 0, 0);
  pg.fill(0, 255, 0);
  pg.ellipse(pg.width/2, pg.height/2, pg.height/3, pg.height/3);
  pg.endDraw();
}

void saveCanvas(String name) {
  //save PGraphics
  pg.save(name + ".png");
}
@AmnonOwed

This comment has been minimized.

Show comment
Hide comment
@AmnonOwed

AmnonOwed Jul 17, 2016

Contributor

Additional findings:

  • The problem also occurs in Processing 3.1
  • A workaround is to actively place the PGraphics content into a PImage and save that.

Workaround Code:

void saveCanvas(String name) {
  // Place PGraphics content in PImage
  PImage img = pg.get();
  //save PImage
  img.save(name + ".png");
}
Contributor

AmnonOwed commented Jul 17, 2016

Additional findings:

  • The problem also occurs in Processing 3.1
  • A workaround is to actively place the PGraphics content into a PImage and save that.

Workaround Code:

void saveCanvas(String name) {
  // Place PGraphics content in PImage
  PImage img = pg.get();
  //save PImage
  img.save(name + ".png");
}
@GoToLoop

This comment has been minimized.

Show comment
Hide comment

GoToLoop commented Jul 20, 2016

@AmnonOwed

This comment has been minimized.

Show comment
Hide comment
@AmnonOwed

AmnonOwed Jul 20, 2016

Contributor

@GoToLoop Thanks for the tip! That works as well.

Personally I would place reliability above speed when it comes to image saving. Especially since it's a silent failure in this case, so you don't even know until you look at the images. Perhaps this hint should be disabled by default until it is more reliable?

Contributor

AmnonOwed commented Jul 20, 2016

@GoToLoop Thanks for the tip! That works as well.

Personally I would place reliability above speed when it comes to image saving. Especially since it's a silent failure in this case, so you don't even know until you look at the images. Perhaps this hint should be disabled by default until it is more reliable?

@GoToLoop

This comment has been minimized.

Show comment
Hide comment
@GoToLoop

GoToLoop Jul 20, 2016

For loading images, Processing got loadImage() and requestImage().
Former is sync and the latter async. Very simple.

IMO, current save() broke the expected behavior big time.
Either make another function or have an extra boolean parameter to change to async mode.

GoToLoop commented Jul 20, 2016

For loading images, Processing got loadImage() and requestImage().
Former is sync and the latter async. Very simple.

IMO, current save() broke the expected behavior big time.
Either make another function or have an extra boolean parameter to change to async mode.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Jul 29, 2016

Member

I'm gonna turn off async image saving as the default in 3.1.2. It's a shame since it's enormously helpful. But there are too many quirks, which means it belongs as a hint().

Member

benfry commented Jul 29, 2016

I'm gonna turn off async image saving as the default in 3.1.2. It's a shame since it's enormously helpful. But there are too many quirks, which means it belongs as a hint().

@benfry benfry closed this Jul 29, 2016

@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar Aug 6, 2016

Contributor

It's for the better to have it as a hint or a separate function.

I was able to reproduce the problem ans I still need to fix it though.

Contributor

JakubValtar commented Aug 6, 2016

It's for the better to have it as a hint or a separate function.

I was able to reproduce the problem ans I still need to fix it though.

@JakubValtar JakubValtar reopened this Aug 6, 2016

@JakubValtar JakubValtar self-assigned this Aug 6, 2016

JakubValtar added a commit to JakubValtar/processing that referenced this issue Aug 6, 2016

Fix resizing targets for async save
Got burned by not setting pixelWidth and pixelHeight properly.

Fixes #4578
@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar Aug 6, 2016

Contributor

Fixed, there was a bug in resizing save targets. If it was a different size it would get cleared to black.

@benfry Are there still any other quirks with async save which I don't know about? Otherwise everything that came up should be handled by now.

Contributor

JakubValtar commented Aug 6, 2016

Fixed, there was a bug in resizing save targets. If it was a different size it would get cleared to black.

@benfry Are there still any other quirks with async save which I don't know about? Otherwise everything that came up should be handled by now.

@benfry benfry closed this in #4607 Aug 6, 2016

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Aug 6, 2016

Member

Fix incorporated for 3.1.3 (or will it be 3.2?)

Member

benfry commented Aug 6, 2016

Fix incorporated for 3.1.3 (or will it be 3.2?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment