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

copy function not working correctly for Java2D renderer #169

Closed
hx2A opened this issue Jan 28, 2021 · 4 comments
Closed

copy function not working correctly for Java2D renderer #169

hx2A opened this issue Jan 28, 2021 · 4 comments

Comments

@hx2A
Copy link
Contributor

hx2A commented Jan 28, 2021

Description

For this code:

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

  // create a simple image with only red pixels
  PImage red_img = createImage(50, 50, RGB);
  color red = color(255, 0, 0);
  red_img.loadPixels();
  for (int i = 0; i < red_img.pixels.length; i++) {
    red_img.pixels[i] = red;
  }
  red_img.updatePixels();
  
  // first red square
  image(red_img, 50, 50);
  // second red square, but only appears for P2D renderer
  copy(red_img, 0, 0, 50, 50, 250, 250, 50, 50);
}

When using the P2D renderer I see two red squares. When using the default renderer I see one. The copy function does not work correctly when the first parameter is a PImage object and I use the JAVA2D renderer.

Expected Behavior

I expect the copy function to work the same for the JAVA2D and P2D renderers.

Current Behavior

This bug appears in 4.0a3 but not in 3.5.4.

Steps to Reproduce

  1. Run the above code
  2. Change the above code to use the default renderer
  3. Run code again, observing the difference

Your Environment

  • Processing version: 4.0a3
  • Operating System and OS version: Linux/Fedora
  • Other information:
    Linux main 5.10.9-201.fc33.x86_64 Java 11, OpenJDK, ANTLR 4, and Travis #1 SMP Wed Jan 20 16:56:23 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
    Fedora release 33 (Thirty Three)

Possible Causes / Solutions

Not sure, the relevant code in PGraphicsJava2D did not change from 3.5.4 to 4.0a3:

  public void copy(PImage src,
                   int sx, int sy, int sw, int sh,
                   int dx, int dy, int dw, int dh) {
    g2.drawImage((Image) src.getNative(),
                 dx, dy, dx + dw, dy + dh,
                 sx, sy, sx + sw, sy + sh, null);
  }
@behreajj
Copy link

behreajj commented Feb 7, 2021

The super class PImage getNative returns null. The JAVA2D renderer would have to do the same work in copy as the subclass PImageAWT in the overridden getNative.

Does the result differ when you use loadImage instead of createImage?

@hx2A
Copy link
Contributor Author

hx2A commented Feb 14, 2021

Interesting - when I replace the createImage call with loadImage, it works correctly for both renderers.

Are you suggesting a fix would be to check the results of the src.getNative() call, and if it isnull, run some extra code?

I'm happy to try this out and submit a PR, but want to know if this is the right approach.

@behreajj
Copy link

Graphics2D.drawImage already checks for null Images. Not only could getNative return null, a sub-class override of getNative could return whatever kind of Object it wished and still match the super-class signature. A single if-check probably would not help you very much. You already know what kind of data type an AWT renderer needs. Below is a hack to work around the issue.

import processing.awt.PGraphicsJava2D;
import java.awt.Color;
import java.awt.Graphics2D;
import java.awt.image.ImageObserver;
import java.awt.image.BufferedImage;
import java.awt.image.WritableRaster;

PGraphicsJava2D renderer;

PImage image;
BufferedImage imgNative;

int sx = 0, sy = 0, sw = 512, sh = 512;
int dx = 32, dy = 64, dw = 256, dh = 256;

void settings() {
  size(512, 512, JAVA2D);
}

void setup() {
  renderer = (PGraphicsJava2D)getGraphics();

  image = createImage(512, 512, ARGB);
  rgb(image);

  imgNative = new BufferedImage(
    image.pixelWidth,
    image.pixelHeight,
    image.format == RGB ? BufferedImage.TYPE_INT_RGB :
    BufferedImage.TYPE_INT_ARGB);
  WritableRaster wr = imgNative.getRaster();
  wr.setDataElements(
    0, 0,
    image.pixelWidth, image.pixelHeight,
    image.pixels);
}

void draw() {
  renderer.g2.drawImage(
    imgNative,
    dx, dy, dx + dw, dy + dh,
    sx, sy, sx + sw, sy + sh,
    (Color)null,
    (ImageObserver)null);
}

PImage rgb ( PImage target ) {
  target.loadPixels();
  int[] px = target.pixels;
  int len = px.length;
  int w = target.width;
  float hInv = 255.0 / ( target.height - 1.0 );
  float wInv = 255.0 / ( w - 1.0 );
  for ( int i = 0; i < len; ++i ) {
    px[i] = 0xff000080 | ( int ) ( 0.5 + wInv * ( i % w ) ) << 0x10
      | ( int ) ( 255.5 - hInv * ( i / w ) ) << 0x8;
  }
  target.updatePixels();
  return target;
}

PApplet's loadImage calls PSurface's loadImage. PSurface is an interface that leaves it to implementations to define the method. PSurfaceNone calls ShimAWT's loadImage, which creates a PImageAWT. PSurfaceAWT extends PSurfaceNone. By contrast, all createImage does is this.

I'm not a Processing dev, so you'd have to ask them about the right approach, whether now is a good time to work on a PR, etc.

@benfry benfry closed this as completed in 5b42803 Jun 15, 2021
@github-actions
Copy link

This issue has been automatically locked. To avoid confusion with reports that have already been resolved, closed issues are automatically locked 30 days after the last comment. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants