[opensource-dev] Fwd: PO Review Build

[Trilo Byte]

Oops - hit reply instead of reply all...
> 
> Checked out on Mac client.
> 
>> STORM-250 [#STORM-250] Unexpected "More" text appears in the About Landmark 
>> panel after minimising the floater - Second Life Bug Tracker 
> Unexpected 'More' text no longer appears, but arrow button (lower right of 
> floater) stretched/distorted.
> 
>> STORM-610 [#STORM-610] Changes to Environment Editor: water color change is 
>> not saved - Second Life Bug Tracker 
> Works as expected.
> 
>> STORM-954 [#STORM-954] SL-viewer 2.0 No nearby people when over 
>> approximately 1000 meters - Second Life Bug Tracker 
> Now works as expected - excellent!
> 
>> STORM-1057 [#STORM-1057] [TRUNCATION/LAYOUT] Make space larger for full 
>> strings to appear in the Move & View Preferences,
> Appears to be fixed/width adjusted.
> 
>> STORM-1098 [#STORM-1098] Debug setting ShowNetStats has wrong description - 
>> Second Life Bug Tracker 
> Description appears to be updated
> 
>> STORM-1101 [#STORM-1101] Using the -login parameter makes some keys and 
>> right click menus not work - Second Life Bug Tracker 
> (I don't use -login parameters)
> 
>> STORM-1109 [#SOCIAL-864] Check box resizes incorrectly if applying word wrap 
>> to its label. - Second Life Bug Tracker 
> No permission to view issue
> 
>> STORM-1110 [#STORM-1110] Remove old 1.23 purple text from alerts, anywhere 
>> else it appears - Second Life Bug Tracker 
> Barney appears to have been vanquished, no purple text found. 
> 
>> STORM-1117 [#STORM-1117] Placing calling cards in prims should be disabled - 
>> Second Life Bug Tracker 
> Works as expected
> 
>> STORM-1118 [#STORM-1118] Viewer crashes when user tries to upload image 
>> without JFIF header - Second Life Bug Tracker 
> Works as expected. Viewer does not crash when uploading example file, but 
> displays Unable to Read Image error message.
> 

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] s3-proxy.lindenlab.com points to a private IP address?

[Celierra Darling]

I'm still seeing both of these, for FMOD and for KDU, in viewer-development
rev 15768 (8b6c61efed4c), the latest revision at the moment.  I can get past
the FMOD step by explicitly specifying  -- -DFMOD:BOOL=FALSE on the
configure command line (suggested by Ima on #opensl), but KDU doesn't seem
to have an option that's so simple.  (I'd rather not edit the URL to
download gray-area files if I can help it.)

Celi


On Thu, Apr 7, 2011 at 7:45 AM, WolfPup Lowenhar wrote:

> Ok I see two things here Glimmering Sands:
>
> 1.  You are using an old repository the current one is actually viewer
> development as autobuild has been merged in.
>
> 2.  FMOD and KDU are actually somewhere in
> http://s3-proxy.lindenlab.com of which we as Open Source Developers have
> NO access to and are NOT allowed to have access to.
>
>
>
> Ima Mecanique,
>
> The FMOD and KDU libs are not distributed externally if you look at a
> current copy of the autobuild.xml you will find that the address is a
> private one for them and a few others LL cannot freely distribute. Now LL
> has ‘provided’ a way for those that have FMOD already to ‘re-pacakage’ this
> one for use in the autobuild. LL dose not actually modify any of the FMOD
> files but actually creates a ‘wraper’ in the form of llaudio to interface
> with fmod.dll so that those that are on Windows will have working sound.
>
>
>
> *From:* opensource-dev-boun...@lists.secondlife.com [mailto:
> opensource-dev-boun...@lists.secondlife.com] *On Behalf Of *Ima Mechanique
> *Sent:* Thursday, April 07, 2011 7:03 AM
> *To:* opensource-dev@lists.secondlife.com
> *Subject:* Re: [opensource-dev] s3-proxy.lindenlab.com points to a private
> IP address?
>
>
>
> > Linden Lab should not be distributing either of these libraries, as
> > they do not have a license that permits them to do so(from what I have
> > heard from them in other dialogs). So this violates and possibly voids
> > their license for Kakadu, as well as all of their careful planning to
> > keep it out of every developers hands except for their own.
> >
> > I hope that they will take these down, and it is not an error that you
> > should NOT be able to download them. The error here is that they are
> > trying to be downloaded at all.
>
> This is something of a grey area.
> For KDU, this is not distributing per se, as it is only the headers and
> lib file to link against, not the final library being made available.
> I'm not privy to the license details so have no idea if this would be a
> breach of terms..
>
> For FMOD, things are clearer.
>
> "The contents of the FMOD distribution archive may not be redistributed,
> reproduced, modified, transmitted, broadcast, published or adapted in any
> way, shape or form, without the prior written consent of the owner,
> Firelight Technologies, be it by tangible or non tangible media.
>
> The fmod.dll file may be redistributed without the authors prior
> permission,
> and must remain unmodified."
>
> LL have both modified the archive (by creating a new one) and are
> redistributing that modified version, so I hope they have obtained
> written permission from Firelight Technologies  I'd hope that FT would
> be happy to grant permission, as it relieves their servers of the
> download bandwidth required.
>
>
> > On Wed, Apr 6, 2011 at 23:34, Glimmering Sands
> >  wrote:
> > > Well, I have discovered, through the wonders of Mr. Google, that there
> > > is an older copy autobuild.xml out there at:
> > >
> > >
> https://bitbucket.org/merov_linden/viewer-autobuild2010/src/44f214103cff/autobuild.xml
> > >
> > > and I was able to simply use the
> > >
> > > http://s3.amazonaws.com/viewer-source-downloads/install_pkgs/...
> > >
> > > lines for both fmod and kdu (would it be considered a bug that Linden
> > > Labs is distributing an autobuild.xml that uses internal only paths
> > > rather than using the external paths (that I guessed the used to
> > > have?)
> > >
> > > One can also wonder why you would let internal DNS records be allowed
> > > to external sites as well...
> > >
> > > Sometime later I will discover if this solves my grey goo box problem
> or not :-)
> > >
> > > Hugs,
> > >
> > > Glimmering
>
>
> --
> Ima Mechanique
> ima.mechanique(at)blueyonder.co.uk
>
> ___
> Policies and (un)subscribe information available here:
> http://wiki.secondlife.com/wiki/OpenSource-Dev
> Please read the policies before posting to keep unmoderated posting
> privileges
> --
>
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 10.0.1209 / Virus Database: 1500/3556 - Release Date: 04/06/11
>
> ___
> Policies and (un)subscribe information available here:
> http://wiki.secondlife.com/wiki/OpenSource-Dev
> Please read the policies before posting to keep unmoderated posting
> privileges
>
___
Policies and (un)subscribe information 

Re: [opensource-dev] Review Request: (STORM-1042) Disabled 'Save' button at the 'Create Landmark' panel

[Seth ProductEngine]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/261/
---

(Updated April 8, 2011, 5 p.m.)


Review request for Viewer.


Summary
---

Fixed the inventory observers of newly added items.

Removed pieces of code marked as fix for DEV-20328 but could not check for the 
regression.


This addresses bug STORM-1042.
http://jira.secondlife.com/browse/STORM-1042


Diffs (updated)
-

  indra/newview/llinventorymodel.h d30636c2a83a 
  indra/newview/llinventorymodel.cpp d30636c2a83a 
  indra/newview/llinventorymodelbackgroundfetch.cpp d30636c2a83a 
  indra/newview/llinventoryobserver.h d30636c2a83a 
  indra/newview/llinventoryobserver.cpp d30636c2a83a 

Diff: http://codereview.secondlife.com/r/261/diff


Testing
---


Thanks,

Seth

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

[opensource-dev] Review Request: (STORM-1042) Disabled 'Save' button at the 'Create Landmark' panel

[Seth ProductEngine]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/261/
---

Review request for Viewer.


Summary
---

Fixed the inventory observers of newly added items.

Removed pieces of code marked as fix for DEV-20328 but could not check for the 
regression.


This addresses bug STORM-1042.
http://jira.secondlife.com/browse/STORM-1042


Diffs
-


Diff: http://codereview.secondlife.com/r/261/diff


Testing
---


Thanks,

Seth

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: STORM-1118 Viewer crashes when user tries to upload image without JFIF header

[Boroondas Gupte]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/255/#review580
---



indra/llimage/llimagedimensionsinfo.h


You probably mean "whether" not "if". (The check is performed in either 
case, just the result will be different.)

Also, "larger or equal min_len bytes" or "at least min_len bytes large" 
might be easier to understand than the negation "not shorter than min_len 
bytes". (Or maybe "long" rather than "large"? Dunno.)

The semantic of the method could be further emphasized by naming it e.g.
bool isFileLengthAtLeast(S32 min_len);
or even
bool isFileLengthAtLeastBytes(S32 min_len);

Lastly, it might be obvious, but it's probably still worth mentioning that 
"the file" is mInfile.



indra/llimage/llimagedimensionsinfo.cpp


sizeof(signature)/sizeof(signature[0]) will always be 2 here, due to the 
line above, won't it? If so, it might be better to do

// Read BMP signature.
const size_t SIGNATURE_SIZE = 2;
U8 signature[SIGNATURE_SIZE];
mInfile.read((void*)signature, SIGNATURE_SIZE);



indra/llimage/llimagedimensionsinfo.cpp


Magic number. (I guess it's BMP header (14) + DIB header - current position 
(File begin + 2)?)



indra/llimage/llimagedimensionsinfo.cpp


8 isn't explained ...



indra/llimage/llimagedimensionsinfo.cpp


... until here. (Assuming it's the same 8.) Might be worth another constant.



indra/llimage/llimagedimensionsinfo.cpp


Wouldn't a seek be a "cheaper" way to determine size than reading into an 
actual buffer? (According to indra/llcommon/llapr.h, it returns -1 on failure.)

There's also a static method LLAPRFile::size, but that seems to operate on 
not-yet-opened files given by filename.


- Boroondas


On April 7, 2011, 4:20 p.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/255/
> ---
> 
> (Updated April 7, 2011, 4:20 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> * Added checks for image file contents not matching the file extension (e.g. 
> a bitmap named file.jpg).
> * Added checks for abnormally short files to avoid crashes when parsing them.
> 
> 
> This addresses bug STORM-1118.
> http://jira.secondlife.com/browse/STORM-1118
> 
> 
> Diffs
> -
> 
>   indra/llimage/llimagedimensionsinfo.h 33ca961b0870 
>   indra/llimage/llimagedimensionsinfo.cpp 33ca961b0870 
> 
> Diff: http://codereview.secondlife.com/r/255/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for ECC-49 / storm-1147: Other avatar's loose clothing\ flared cuffs look like they are tight\got no flare in v.2.4.0 and higher

[ardylay]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/260/#review581
---

Ship it!


I started using this daily about a week ago.  It does the trick and I have 
noticed NO adverse affects.

- ardy.lay


On April 8, 2011, 2:22 p.m., Xiaohong Bao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/260/
> ---
> 
> (Updated April 8, 2011, 2:22 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This patch is a resident posted for ECC-49. It is correct and works fine. So 
> please merge it into viewer-development and viewer-release.
> 
> 
> This addresses bugs /, ECC-49 and storm-1147.
> http://jira.secondlife.com/browse//
> http://jira.secondlife.com/browse/ECC-49
> http://jira.secondlife.com/browse/storm-1147
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewertexture.cpp c1559a7da393 
> 
> Diff: http://codereview.secondlife.com/r/260/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Xiaohong
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for STORM-973: [crashhunters] crash at LLViewerTextureList::removeImageFromList(LLViewerFetchedTexture *)7

[Boroondas Gupte]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/252/#review567
---


I guess there is no real reason why there's a space before some semicolons but 
not others, or is there? Even although I like to look with the space there, I 
think we should remove those spaces to be consistent with common practice.


indra/newview/llviewertexturelist.h


Any reason for this to be BOOL instead of bool?
Also, remove the space between the name and the semicolon.



indra/newview/llviewertexturelist.cpp


Remove space between value and semicolon.



indra/newview/llviewertexturelist.cpp


I guess the intention is that this gets only executed by one single thread. 
(And probably only once?) So before setting sRenderThreadID to the current 
thread's ID, we might want to assert that it hasn't been changed before (i.e., 
is still 0).

(If we want to allow to execute this assignment several times but only by 
one thread, check whether it's 0 or already has the value of 
LLThread::currentID().)



indra/newview/llviewertexturelist.cpp


Remove space before semicolon.



indra/newview/llviewertexturelist.cpp


Same here.



indra/newview/llviewertexturelist.cpp


And while we're at it, here, too.



indra/newview/llviewertexturelist.cpp


And there's more of these spaces. (I won't comment on all.)


- Boroondas


On April 8, 2011, 10:26 a.m., Xiaohong Bao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/252/
> ---
> 
> (Updated April 8, 2011, 10:26 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> this is to resubmit the patch for storm-973.
> 
> We are not very clear what causes this. But this fix is targeting three most 
> possible causes:
> 1, a texture is failed to add into mImageList but its flag is set to be 
> successful;
> 2, a texture status is changed not from the main thread, because gTextureList 
> is not thread-safe;
> 3, gTextureList is accessed before it is initialized.
> 
> I regenerated the viewer-development-storm-973 branch based on the latest 
> viewer-development branch. If you still can not apply the patch directly, I 
> am afraid you should do the manual merge. Otherwise grant me the permission, 
> I will do it. 
> 
> 
> This addresses bug storm-973.
> http://jira.secondlife.com/browse/storm-973
> 
> 
> Diffs
> -
> 
>   indra/newview/lldrawpoolbump.cpp 13670741a0a8 
>   indra/newview/llviewertexturelist.h 13670741a0a8 
>   indra/newview/llviewertexturelist.cpp 13670741a0a8 
> 
> Diff: http://codereview.secondlife.com/r/252/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Xiaohong
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for ECC-49 / storm-1147: Other avatar's loose clothing\ flared cuffs look like they are tight\got no flare in v.2.4.0 and higher

[Boroondas Gupte]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/260/#review579
---


Don't forget to credit the Residents providing the fix in doc/contributions.txt.

- Boroondas


On April 8, 2011, 2:22 p.m., Xiaohong Bao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/260/
> ---
> 
> (Updated April 8, 2011, 2:22 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This patch is a resident posted for ECC-49. It is correct and works fine. So 
> please merge it into viewer-development and viewer-release.
> 
> 
> This addresses bugs /, ECC-49 and storm-1147.
> http://jira.secondlife.com/browse//
> http://jira.secondlife.com/browse/ECC-49
> http://jira.secondlife.com/browse/storm-1147
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewertexture.cpp c1559a7da393 
> 
> Diff: http://codereview.secondlife.com/r/260/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Xiaohong
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

[opensource-dev] PO Review Build

[Oz Linden (Scott Lawrence)]

http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/oz_viewer-poreview/rev/226034/index.html

STORM-250  [#STORM-250] 
Unexpected "More" text appears in the About Landmark panel after 
minimising the floater - Second Life Bug Tracker
STORM-610  [#STORM-610] 
Changes to Environment Editor: water color change is not saved - Second 
Life Bug Tracker
STORM-954  [#STORM-954] 
SL-viewer 2.0 No nearby people when over approximately 1000 meters - 
Second Life Bug Tracker
STORM-1057  [#STORM-1057] 
[TRUNCATION/LAYOUT] Make space larger for full strings to appear in the 
Move & View Preferences, ALL langs - Second Life Bug Tracker
STORM-1098  [#STORM-1098] 
Debug setting ShowNetStats has wrong description - Second Life Bug Tracker
STORM-1101  [#STORM-1101] 
Using the -login parameter makes some keys and right click menus not 
work - Second Life Bug Tracker
STORM-1109  [#SOCIAL-864] 
Check box resizes incorrectly if applying word wrap to its label. - 
Second Life Bug Tracker
STORM-1110  [#STORM-1110] 
Remove old 1.23 purple text from alerts, anywhere else it appears - 
Second Life Bug Tracker
STORM-1117  [#STORM-1117] 
Placing calling cards in prims should be disabled - Second Life Bug Tracker
STORM-1118  [#STORM-1118] 
Viewer crashes when user tries to upload image without JFIF header - 
Second Life Bug Tracker


___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

[opensource-dev] Review Request: fix for ECC-49 / storm-1147: Other avatar's loose clothing\ flared cuffs look like they are tight\got no flare in v.2.4.0 and higher

[Xiaohong Bao]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/260/
---

Review request for Viewer.


Summary
---

This patch is a resident posted for ECC-49. It is correct and works fine. So 
please merge it into viewer-development and viewer-release.


This addresses bugs /, ECC-49 and storm-1147.
http://jira.secondlife.com/browse//
http://jira.secondlife.com/browse/ECC-49
http://jira.secondlife.com/browse/storm-1147


Diffs
-

  indra/newview/llviewertexture.cpp c1559a7da393 

Diff: http://codereview.secondlife.com/r/260/diff


Testing
---


Thanks,

Xiaohong

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for STORM-973: [crashhunters] crash at LLViewerTextureList::removeImageFromList(LLViewerFetchedTexture *)7

[Vadim ProductEngine]

> On April 8, 2011, 12:53 p.m., Vadim ProductEngine wrote:
> >
> 
> Vadim ProductEngine wrote:
> > I regenerated the viewer-development-storm-973 branch based on the 
> latest viewer-development branch.
> > If you still can not apply the patch directly, I am afraid you should 
> do the manual merge. Otherwise grant me the permission, I will do it. 
> Manual merge with what? I just applied the patch to the revision it was 
> made for. There were no conflicting changes.
> Let me elaborate...
> The patch claims to have been generated on top of changeset 13670741a0a8, 
> which doesn't have the "if(gNoRender) return;" line in 
> LLViewerTextureList::decodeAllImages(), thus the patch fails to apply.
> I'm pretty sure there's something wrong with the way the patch was 
> generated.
> 
> BTW, viewer-development and viewer-development-storm-973 are absolutely 
> identical. What's the point of having an identical clone?
> 
> Vadim ProductEngine wrote:
> I was wrong. The patch was made for changeset 8395569a8f77, which is 
> missing in both repositories. That must be the reason...

Oops. The first comment is correct. The patch indeed was made for 13670741a0a8.


- Vadim


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/252/#review575
---


On April 8, 2011, 10:26 a.m., Xiaohong Bao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/252/
> ---
> 
> (Updated April 8, 2011, 10:26 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> this is to resubmit the patch for storm-973.
> 
> We are not very clear what causes this. But this fix is targeting three most 
> possible causes:
> 1, a texture is failed to add into mImageList but its flag is set to be 
> successful;
> 2, a texture status is changed not from the main thread, because gTextureList 
> is not thread-safe;
> 3, gTextureList is accessed before it is initialized.
> 
> I regenerated the viewer-development-storm-973 branch based on the latest 
> viewer-development branch. If you still can not apply the patch directly, I 
> am afraid you should do the manual merge. Otherwise grant me the permission, 
> I will do it. 
> 
> 
> This addresses bug storm-973.
> http://jira.secondlife.com/browse/storm-973
> 
> 
> Diffs
> -
> 
>   indra/newview/lldrawpoolbump.cpp 13670741a0a8 
>   indra/newview/llviewertexturelist.h 13670741a0a8 
>   indra/newview/llviewertexturelist.cpp 13670741a0a8 
> 
> Diff: http://codereview.secondlife.com/r/252/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Xiaohong
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for STORM-973: [crashhunters] crash at LLViewerTextureList::removeImageFromList(LLViewerFetchedTexture *)7

[Vadim ProductEngine]

> On April 8, 2011, 12:53 p.m., Vadim ProductEngine wrote:
> >
> 
> Vadim ProductEngine wrote:
> > I regenerated the viewer-development-storm-973 branch based on the 
> latest viewer-development branch.
> > If you still can not apply the patch directly, I am afraid you should 
> do the manual merge. Otherwise grant me the permission, I will do it. 
> Manual merge with what? I just applied the patch to the revision it was 
> made for. There were no conflicting changes.
> Let me elaborate...
> The patch claims to have been generated on top of changeset 13670741a0a8, 
> which doesn't have the "if(gNoRender) return;" line in 
> LLViewerTextureList::decodeAllImages(), thus the patch fails to apply.
> I'm pretty sure there's something wrong with the way the patch was 
> generated.
> 
> BTW, viewer-development and viewer-development-storm-973 are absolutely 
> identical. What's the point of having an identical clone?

I was wrong. The patch was made for changeset 8395569a8f77, which is missing in 
both repositories. That must be the reason...


- Vadim


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/252/#review575
---


On April 8, 2011, 10:26 a.m., Xiaohong Bao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/252/
> ---
> 
> (Updated April 8, 2011, 10:26 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> this is to resubmit the patch for storm-973.
> 
> We are not very clear what causes this. But this fix is targeting three most 
> possible causes:
> 1, a texture is failed to add into mImageList but its flag is set to be 
> successful;
> 2, a texture status is changed not from the main thread, because gTextureList 
> is not thread-safe;
> 3, gTextureList is accessed before it is initialized.
> 
> I regenerated the viewer-development-storm-973 branch based on the latest 
> viewer-development branch. If you still can not apply the patch directly, I 
> am afraid you should do the manual merge. Otherwise grant me the permission, 
> I will do it. 
> 
> 
> This addresses bug storm-973.
> http://jira.secondlife.com/browse/storm-973
> 
> 
> Diffs
> -
> 
>   indra/newview/lldrawpoolbump.cpp 13670741a0a8 
>   indra/newview/llviewertexturelist.h 13670741a0a8 
>   indra/newview/llviewertexturelist.cpp 13670741a0a8 
> 
> Diff: http://codereview.secondlife.com/r/252/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Xiaohong
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for STORM-973: [crashhunters] crash at LLViewerTextureList::removeImageFromList(LLViewerFetchedTexture *)7

[Vadim ProductEngine]

> On April 8, 2011, 12:53 p.m., Vadim ProductEngine wrote:
> >

> I regenerated the viewer-development-storm-973 branch based on the latest 
> viewer-development branch.
> If you still can not apply the patch directly, I am afraid you should do the 
> manual merge. Otherwise grant me the permission, I will do it. 
Manual merge with what? I just applied the patch to the revision it was made 
for. There were no conflicting changes.
Let me elaborate...
The patch claims to have been generated on top of changeset 13670741a0a8, which 
doesn't have the "if(gNoRender) return;" line in 
LLViewerTextureList::decodeAllImages(), thus the patch fails to apply.
I'm pretty sure there's something wrong with the way the patch was generated.

BTW, viewer-development and viewer-development-storm-973 are absolutely 
identical. What's the point of having an identical clone?


- Vadim


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/252/#review575
---


On April 8, 2011, 10:26 a.m., Xiaohong Bao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/252/
> ---
> 
> (Updated April 8, 2011, 10:26 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> this is to resubmit the patch for storm-973.
> 
> We are not very clear what causes this. But this fix is targeting three most 
> possible causes:
> 1, a texture is failed to add into mImageList but its flag is set to be 
> successful;
> 2, a texture status is changed not from the main thread, because gTextureList 
> is not thread-safe;
> 3, gTextureList is accessed before it is initialized.
> 
> I regenerated the viewer-development-storm-973 branch based on the latest 
> viewer-development branch. If you still can not apply the patch directly, I 
> am afraid you should do the manual merge. Otherwise grant me the permission, 
> I will do it. 
> 
> 
> This addresses bug storm-973.
> http://jira.secondlife.com/browse/storm-973
> 
> 
> Diffs
> -
> 
>   indra/newview/lldrawpoolbump.cpp 13670741a0a8 
>   indra/newview/llviewertexturelist.h 13670741a0a8 
>   indra/newview/llviewertexturelist.cpp 13670741a0a8 
> 
> Diff: http://codereview.secondlife.com/r/252/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Xiaohong
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for STORM-973: [crashhunters] crash at LLViewerTextureList::removeImageFromList(LLViewerFetchedTexture *)7

[Vadim ProductEngine]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/252/#review575
---



indra/newview/llviewertexturelist.cpp


Almost every line of this method is a potential crasher.
Are you sure we should handle errors this way?

I would:
 * Add a NULL check for the "image" parameter (besides the llassert which 
is not evaluated in release builds).
 * Try handling the other errors without crashing, by issuing a warning and 
returning FALSE.

Otherwise we may fix one crash and add two.



indra/newview/llviewertexturelist.cpp


ditto



indra/newview/llviewertexturelist.cpp


* No check for mInitialized before accessing sRenderThreadID.
* I don't quite get what you're trying to achieve here by passing 
sRenderThreadID. It doesn't guarantee that the method is invoked from thread 
#, does it?


- Vadim


On April 8, 2011, 10:26 a.m., Xiaohong Bao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/252/
> ---
> 
> (Updated April 8, 2011, 10:26 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> this is to resubmit the patch for storm-973.
> 
> We are not very clear what causes this. But this fix is targeting three most 
> possible causes:
> 1, a texture is failed to add into mImageList but its flag is set to be 
> successful;
> 2, a texture status is changed not from the main thread, because gTextureList 
> is not thread-safe;
> 3, gTextureList is accessed before it is initialized.
> 
> I regenerated the viewer-development-storm-973 branch based on the latest 
> viewer-development branch. If you still can not apply the patch directly, I 
> am afraid you should do the manual merge. Otherwise grant me the permission, 
> I will do it. 
> 
> 
> This addresses bug storm-973.
> http://jira.secondlife.com/browse/storm-973
> 
> 
> Diffs
> -
> 
>   indra/newview/lldrawpoolbump.cpp 13670741a0a8 
>   indra/newview/llviewertexturelist.h 13670741a0a8 
>   indra/newview/llviewertexturelist.cpp 13670741a0a8 
> 
> Diff: http://codereview.secondlife.com/r/252/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Xiaohong
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: STORM-1118 Viewer crashes when user tries to upload image without JFIF header

[Seth ProductEngine]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/255/#review574
---

Ship it!


Looks good to me.

- Seth


On April 7, 2011, 4:20 p.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/255/
> ---
> 
> (Updated April 7, 2011, 4:20 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> * Added checks for image file contents not matching the file extension (e.g. 
> a bitmap named file.jpg).
> * Added checks for abnormally short files to avoid crashes when parsing them.
> 
> 
> This addresses bug STORM-1118.
> http://jira.secondlife.com/browse/STORM-1118
> 
> 
> Diffs
> -
> 
>   indra/llimage/llimagedimensionsinfo.h 33ca961b0870 
>   indra/llimage/llimagedimensionsinfo.cpp 33ca961b0870 
> 
> Diff: http://codereview.secondlife.com/r/255/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Windows compiling problem

[Jonathan Welch]

I've tried all the suggestions that people have suggested, but to no
avail.  I still get messages like the one below.

Anyone with more ideas?

-- Build started: Project: llwindow, Configuration: Release Win32 --
  llwindowwin32.cpp
  lldxhardware.cpp
e:\Microsoft SDKs\Windows\v7.1\Include\objidl.h(11280): error C2061:
syntax error : identifier '__RPC__out_xcount_part'
e:\Microsoft SDKs\Windows\v7.1\Include\objidl.h(11281): error C2059:
syntax error : ')'
e:\Microsoft SDKs\Windows\v7.1\Include\objidl.h(11281): fatal error
C1903: unable to recover from previous error(s); stopping compilation
___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: KDU Improvements: Compress j2c with precincts

[Vadim ProductEngine]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/256/#review573
---

Ship it!


Not that I understand much about this KDU stuff... but I have no objections, 
apart from mixing relevant changes with cleanups and whitespace changes.


indra/integration_tests/llimage_libtest/llimage_libtest.cpp


memory leak?


- Vadim


On April 7, 2011, 6:01 p.m., Merov Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/256/
> ---
> 
> (Updated April 7, 2011, 6:01 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This patch adds code to j2c to:
> - compress images with the use of precincts and blocks
> - use different ordering (RPCL) and markers in the codestream
> - allow loading of images with region constraint
> 
> All this code can be exercised with a new set of arguments using 
> llimage_libtest. For the moment, the viewer is still saving images with the 
> old precinct and ordering. 
> 
> Things done in this patch:
> - Add arguments to llimage_libtest:
> -- on j2c output : --precincts, --blocks
> -- on j2c input : --region, --discard_level
> - Add InitEncode() and InitDecode() methods to llimagej2c (and derived 
> classes) to implement new input and output parameters
> - Fix perf analyzing report output in llimage_libtest (was broken)
> - General llkdu code clean up: fixed all tab/white space issues, suppress 
> code that was unused, enforced coding conventions, use proper Kakadu method 
> to get its version number
> 
> 
> This addresses bug STORM-746.
> http://jira.secondlife.com/browse/STORM-746
> 
> 
> Diffs
> -
> 
>   indra/integration_tests/llimage_libtest/llimage_libtest.cpp d30636c2a83a 
>   indra/llimage/llimagej2c.h d30636c2a83a 
>   indra/llimage/llimagej2c.cpp d30636c2a83a 
>   indra/llimagej2coj/llimagej2coj.h d30636c2a83a 
>   indra/llimagej2coj/llimagej2coj.cpp d30636c2a83a 
>   indra/llkdu/llimagej2ckdu.h d30636c2a83a 
>   indra/llkdu/llimagej2ckdu.cpp d30636c2a83a 
>   indra/llkdu/llkdumem.h d30636c2a83a 
>   indra/llkdu/llkdumem.cpp d30636c2a83a 
> 
> Diff: http://codereview.secondlife.com/r/256/diff
> 
> 
> Testing
> ---
> 
> Tested only on Mac. All the precincts code can be exercised through 
> llimage_libtest (see ./llimage_libtest --help for details on the new 
> arguments)
> 
> None of the precincts/blocks creation is used in the viewer though, since 
> viewer code is modified, it would be good to test texture downloading, 
> uploading, j2c creation (snapshots and uploads) and all kind of texture 
> loading (sculpties, with and without alphas, small textures, big ones, etc...)
> 
> 
> Thanks,
> 
> Merov
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for STORM-973: [crashhunters] crash at LLViewerTextureList::removeImageFromList(LLViewerFetchedTexture *)7

[Xiaohong Bao]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/252/
---

(Updated April 8, 2011, 10:26 a.m.)


Review request for Viewer.


Changes
---

I changed the description of the review: added the possible causes of this bug.

I regenerated the viewer-development-storm-973 branch based on the latest 
viewer-development branch. If you still can not apply the patch directly, I am 
afraid you should do the manual merge. Otherwise grant me the permission, I 
will do it. 


Summary (updated)
---

this is to resubmit the patch for storm-973.

We are not very clear what causes this. But this fix is targeting three most 
possible causes:
1, a texture is failed to add into mImageList but its flag is set to be 
successful;
2, a texture status is changed not from the main thread, because gTextureList 
is not thread-safe;
3, gTextureList is accessed before it is initialized.

I regenerated the viewer-development-storm-973 branch based on the latest 
viewer-development branch. If you still can not apply the patch directly, I am 
afraid you should do the manual merge. Otherwise grant me the permission, I 
will do it. 


This addresses bug storm-973.
http://jira.secondlife.com/browse/storm-973


Diffs
-

  indra/newview/lldrawpoolbump.cpp 13670741a0a8 
  indra/newview/llviewertexturelist.h 13670741a0a8 
  indra/newview/llviewertexturelist.cpp 13670741a0a8 

Diff: http://codereview.secondlife.com/r/252/diff


Testing
---


Thanks,

Xiaohong

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for STORM-973: [crashhunters] crash at LLViewerTextureList::removeImageFromList(LLViewerFetchedTexture *)7

[Vadim ProductEngine]

> On April 8, 2011, 10:08 a.m., Vadim ProductEngine wrote:
> > A description would not hurt:
> > * What caused the bug?
> > * What fixes it?

Ah, and the fix fails to apply cleanly to viewer-development AND to 
BaoLinden/viewer-development-storm-973.


- Vadim


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/252/#review571
---


On April 5, 2011, 2:32 p.m., Xiaohong Bao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/252/
> ---
> 
> (Updated April 5, 2011, 2:32 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> this is to resubmit the patch for storm-973.
> 
> 
> This addresses bug storm-973.
> http://jira.secondlife.com/browse/storm-973
> 
> 
> Diffs
> -
> 
>   indra/newview/lldrawpoolbump.cpp 13670741a0a8 
>   indra/newview/llviewertexturelist.h 13670741a0a8 
>   indra/newview/llviewertexturelist.cpp 13670741a0a8 
> 
> Diff: http://codereview.secondlife.com/r/252/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Xiaohong
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: fix for STORM-973: [crashhunters] crash at LLViewerTextureList::removeImageFromList(LLViewerFetchedTexture *)7

[Vadim ProductEngine]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/252/#review571
---


A description would not hurt:
* What caused the bug?
* What fixes it?

- Vadim


On April 5, 2011, 2:32 p.m., Xiaohong Bao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/252/
> ---
> 
> (Updated April 5, 2011, 2:32 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> this is to resubmit the patch for storm-973.
> 
> 
> This addresses bug storm-973.
> http://jira.secondlife.com/browse/storm-973
> 
> 
> Diffs
> -
> 
>   indra/newview/lldrawpoolbump.cpp 13670741a0a8 
>   indra/newview/llviewertexturelist.h 13670741a0a8 
>   indra/newview/llviewertexturelist.cpp 13670741a0a8 
> 
> Diff: http://codereview.secondlife.com/r/252/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Xiaohong
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: OPEN-61 Adding locations that VC redistributable package installs files.

[Ima Mechanique]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/259/
---

(Updated April 8, 2011, 7:47 a.m.)


Review request for Viewer.


Summary (updated)
---

The VC redistributable package installs its files (msvcr100.dll, msvcp100.dll, 
msvcr100d.dll, msvcp100d.dll, etc.) to the WINDOWS\System32 directory (and 
WINDOWS\SysWOW64, if it is a 64-bit version). Adding these locations to the 
list of places to look when attempting to copy the files to 
build-vc100\sharedlibs.

On 64 bit windows the 32-bit file versions go to SySWOW64, so this is listed 
first to prevent the 64-bit versions being used.

If any one knows how it could use %systemroot% instead of the environment 
variable WINDIR I'd appreciate the info ;-) 


This addresses bug OPEN-61.
http://jira.secondlife.com/browse/OPEN-61


Diffs
-

  indra/cmake/Copy3rdPartyLibs.cmake 33ca961b0870 

Diff: http://codereview.secondlife.com/r/259/diff


Testing
---

Ran 'autobuild build' without manually copying the redistributable files or 
adding the configure switch to point at their location.


Thanks,

Ima

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: OPEN-61 Adding locations that VC redistributable package installs files.

[Ima Mechanique]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/259/
---

(Updated April 8, 2011, 7:43 a.m.)


Review request for Viewer.


Summary (updated)
---

The VC redistributable package installs its files (msvcr100.dll, msvcp100.dll, 
msvcr100d.dll, msvcp100d.dll, etc.) to the WINDOWS\System32 directory (and 
WINDOWS\SysWOW64, if it is a 64-bit version). Adding these locations to the 
list of places to look when attempting to copy the files to 
build-vc100\sharedlibs.

On 64 bit windows the 32-bit file versions go to SySWOW64, so this is listed 
first to prevent the 64-bit versions being used.


This addresses bug OPEN-61.
http://jira.secondlife.com/browse/OPEN-61


Diffs
-

  indra/cmake/Copy3rdPartyLibs.cmake 33ca961b0870 

Diff: http://codereview.secondlife.com/r/259/diff


Testing
---

Ran 'autobuild build' without manually copying the redistributable files or 
adding the configure switch to point at their location.


Thanks,

Ima

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

[opensource-dev] Review Request: OPEN-54: Making CMake variable name for JsonCpp include dir(s) consistent

[Boroondas Gupte]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/257/
---

Review request for Viewer.


Summary
---

Chose the singular form as that seems to be what most of the Find*.cmake files 
use for their *_INCLUDE_DIR[S] variables.


This addresses bug OPEN-54.
http://jira.secondlife.com/browse/OPEN-54


Diffs
-

  doc/contributions.txt 9ff8625cadec 
  indra/cmake/JsonCpp.cmake 9ff8625cadec 
  indra/newview/CMakeLists.txt 9ff8625cadec 

Diff: http://codereview.secondlife.com/r/257/diff


Testing
---

None, except for grep'ing around to make sure there's now only a single name 
for this.


Thanks,

Boroondas

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges