GD

GD and GD2 are useful libraries for creating graphics on-the-fly. Discuss your PHP GD and GD2 scripts here.

Moderators: onion2k, General Moderators

Post Reply
bob_the _builder
Forum Contributor
Posts: 131
Joined: Sat Aug 28, 2004 12:25 am

GD

Post by bob_the _builder »

Hi,

I am trying to add a border to my images as they are uploaded. The code I have created seems to work but creates a file on the server with names similar to:

Resource id #18

Inside the file is:

Code: Select all

ÿØÿà
ÿÛ
ÿÄ
%&'()*456789:CDEFGHIJSTUVWXYZcdefghijstuvwxyzƒ„…†‡ˆ‰Š’“”•–—˜™š¢£¤¥¦§¨©ª²³´µ¶·¸¹ºÂÃÄÅÆÇÈÉÊÒÓÔÕÖרÙÚáâãäåæçèéêñòó
ôõö÷øùúÿÄ
ÿÄ
$4á%ñ&'()*56789:CDEFGHIJSTUVWXYZcdefghijstuvwxyz‚ƒ„…†‡ˆ‰Š’“”•–—˜™š¢£¤¥¦§¨©ª²³´µ¶·¸¹ºÂÃÄÅÆÇÈÉÊÒÓÔÕÖרÙÚâãäåæçèéêò
óôõö÷øùúÿÚ
Ì+å…
User avatar
onion2k
Jedi Mod
Posts: 5263
Joined: Tue Dec 21, 2004 5:03 pm
Location: usrlab.com

Post by onion2k »

This line is the problem: ImageJpeg($canvas,$piece,100);

$piece isn't a file name, it's an internal resource reference. In this case it's to resource #18. The stuff in the file is a JPEG of whatever $canvas represented at that point in the script.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Choose the Right Board

Post by s.dot »

Moved to the GD forum.
[url=http://forums.devnetwork.net/viewtopic.php?t=30037]Forum Rules[/url] Section 1.1 wrote:1. Select the correct board for your query. Take some time to read the guidelines in the sticky topic.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
bob_the _builder
Forum Contributor
Posts: 131
Joined: Sat Aug 28, 2004 12:25 am

Post by bob_the _builder »

Hi,

Sorry for posting in wrong area.

Thanks for that .. There was a double up of imagejpeg()

Is the code generally written okay or could it be written better?

Thanks
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Post by pickle »

Seems to be formatted pretty well. I can't really see any way to improve it other than abstracting out some of the properties, such as the width & height, the thumbnail prefix & the thumbnail quality. Unless this is something you'll use elsewhere or other people will use though, it's not really necessary.

Also, you don't need to make the thumbnails at 100% quality - especially when they're only 50 x 50px. You can very safely cut the quality down to 75% without any noticeable degradation in image quality, but with a great saving in file size. At such a low resolution, you might even be able to cut it down to 60% or 50%.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
onion2k
Jedi Mod
Posts: 5263
Joined: Tue Dec 21, 2004 5:03 pm
Location: usrlab.com

Post by onion2k »

It's pretty awful to be honest:

1. You're not checking things work correctly. You're not bothering to check an incoming image actually is a JPEG before loading it with imagecreatefromjpeg() for example.

2. You're not catching errors ... Should someone try to load a PNG file for example the script will fail without doing anything about telling you or creating a placeholder image.

3. You're opening the same image twice. That's a waste of time, especially for a CPU hungry function like imagecreatefromjpeg(). Open it once and reuse it.

4. You're populating an array with the details of the image you're loading ($dimensions) but then you're not using the information. That's wasteful too.

5. You've hardcoded parts of filenames (the "tb_" bit). If you (or the client) decide that you want them to be called something different you'll have to edit the script, or multiple scripts if you're using the same code elsewhere.

6. You're opening a jpeg, resizing it, overwriting the original, then opening it again, adding borders, then overwriting it again. It's a waste of time, you might as well remove the step where you save it in the middle. If you really have to save an image that you're only going to use in a PHP script it's preferable to use imagegd2() to save it, the gd2 format is much quicker to open making your script more efficient.

7. You're using 100 quality on imagejpeg() on the final output image. There's very little difference in quality from, say, 80 but there's a pretty big difference in file size. You're wasting bandwidth and disk space.
woozy
Forum Newbie
Posts: 2
Joined: Sun Jan 27, 2008 9:24 pm

Re:

Post by woozy »

onion2k wrote: 1. You're not checking things work correctly. You're not bothering to check an incoming image actually is a JPEG before loading it with imagecreatefromjpeg() for example.
Forgive this newbie and his newbie question but howcan one check this? I've been doing shortcuts like checking file extensions and running through PHP's is_file() function, and trusting that the scripts I write only refer to actual jpegs but I haven't been doing anything resembling actual validation.

BTW, is there a basic FAQ file about *how* GD works including, briefly, what exactly is the image data stored in a var $im = createfromjpeg($file) command and things like that? I don't need to know any of this but I'm finding myself very curious.

-woozy
Post Reply