Hi Pierre,

Your comments are correct, but I would add this into the mix --

the two chunks of code are functionally equivalent, but the original code layout is probably easier to debug, simply from the point of view of stepping through with a debugger.  More a stylistic choice than anything else.

Ciao,

marks


On 2/7/06, Pierre Dumuid <[EMAIL PROTECTED]> wrote:
From what I can see, (acting as a third party reviewer), the additional code that has been introduced only turns of the pre-existing conversion operations
for the pixels exceeding the diagonal, (as determined by condition of j + i*(w/h) < w ) .  Concerning if it worked for all color model's and under strange circumstances.   I personally would consider that the histogram would works no less than it would have before. 

The only other comment I would have is to remove the substraction of the white space in histogramwindow.C, and possibly reduce
+			if (plugin->config.split) \
+ { \
+ if ((j + i * w / h) < w) \
+ continue; \
+ } \

to

+ if ( plugin->config.split && ((j + i * w / h) < w) ) \
+ continue; \
I am fairly sure they are equivalent, as if the test preceding the && is false, I don't think the following test is examined.  Not 100% though.

These changes would make the differences between upstream and svn smaller.  I could possibly try the patch tonight and apply it.  Andraz, are you happy if I do that?

Pierre


Andraz Tori wrote:
it was not eaten, i just didn't have time...

could you please also give an assessement on quality of the code.. so is
it ready for production - has it been tested in all color models and
under 'strange circumstances'...

?

bye
andraž

On pon, 2006-02-06 at 23:38 +0100, Jonas Wulff wrote:
Hello everyone,

since my last e-mail regarding this small patch of mine I neither got
any comments nor saw it being commited to SVN, so I guess it has been
eaten by some spam-filters (likely, as there was something wrong with my
server setup)...
So I'm broadcasting my patch again (this time as svn-diff), hoping I
don't annoy somebody and that I get some comments...

(Short description: it adds an extra button to the histogram plugin to
split the output picture into one half that is color-corrected and
another that is not.)

Thanks for your time,
Jonas
_______________________________________________
Cinelerra mailing list
Cinelerra@skolelinux.no
https://init.linpro.no/mailman/skolelinux.no/listinfo/cinelerra


-- 
__________________________________________________________________

Get Firefox! http://www.mozilla.org/products/firefox/

Mr Pierre Dumuid
Phd Student

Active Noise and Vibration Control Group
School of Mechanical Engineering
The University of Adelaide
SA 5005
AUSTRALIA

Email: [EMAIL PROTECTED]
[EMAIL PROTECTED]
Work: +61 (0)8 8303 3847

Mobile: 0407 570 263

Web Page : http://www.mecheng.adelaide.edu.au/
__________________________________________________________________

CRICOS Provider Number 00123M
-----------------------------------------------------------

This email message is intended only for the addressee(s) and contains
information that may be confidential and/or copyright. If you are not
the intended recipient please notify the sender by reply email and
immediately delete this email. Use, disclosure or reproduction of this
email by anyone other than the intended recipient(s) is strictly
prohibited. No representation is made that this email or any attachments
are free of viruses. Virus scanning is recommended and is the
responsibility of the recipient.







--
Mark Stavar

Swan Dancer Productions

Email:  [EMAIL PROTECTED]
Mobile: 0410 638 671

Reply via email to